Previous 5 version of ARM OPTPROBES patches are unable to deal with
stack storing instructions correctly. V5 patches disallow optimizing
every protential stack store instructions based on pessimistic
assumption. Which, as Tixy comments, 'excludes the main use of
kprobes'. (https://lkml.org/lkml/2014/8/29/117 )
The main obstacle which prevents us from computing stack requirement is
the missing of per-instruction decoder in probes_decode_insn() and its
friends. Only part of instructions have their decoders (and not in
each case).
In this patch series, I propose 'checker', which allows us define
functions for each type of instruction, extract more information. Stack
consumption computing is an example. Checker can be further employed to
determine whether one instruction is possible to execute directy in
optimized kprobe. I'd like to expand current checker framework by
chaining checkers together. After that, I believe most of ARM
instructions can be executed directly like x86, kprobe performace can be
improved.
The first 3 patches introduces checker. After that, patch 4/7 checks
stack requirement for probed instructions. Patches 5/7 - 7/7 are similar
to patch v5, except:
1. As Tixy proposed, unoptimized probes are also suffer from stack
problem (https://lkml.org/lkml/2014/9/1/548 ). Commit d30a0c8b saves
64 bytes for them, but for instruction use register addressing (like
'str r0, [sp, r1]'), 64 bytes are unsafe. Patch 5/7 prohibit such
probing according to stack information collected by checker.
2. In patch 7/7, stack protection code now is generated according to
the instruction be optimized.
3. In patch 7/7, kprobes-opt.c is renamed to kprobes-opt-arm.c due to
it only deal with ARM case.
4. Bug in v5 is fixed.
Wang Nan (7):
ARM: kprobes: replace 'union decode_action' to 'struct decode_action'
ARM: kprobes: seprates load and store actions
ARM: kprobes: introduces checker
ARM: kprobes: collects stack consumption for store instructions
ARM: kprobes: disallow probing stack consuming instructions
kprobes: copy ainsn after alloc aggr kprobe
ARM: kprobes: enable OPTPROBES for ARM 32
arch/arm/Kconfig | 1 +
arch/arm/include/asm/kprobes.h | 26 ++++
arch/arm/include/asm/probes.h | 1 +
arch/arm/kernel/Makefile | 3 +-
arch/arm/kernel/kprobes-arm.c | 12 +-
arch/arm/kernel/kprobes-opt-arm.c | 285 +++++++++++++++++++++++++++++++++++
arch/arm/kernel/kprobes-test-arm.c | 17 ++-
arch/arm/kernel/kprobes-test-thumb.c | 13 ++
arch/arm/kernel/kprobes-thumb.c | 24 +--
arch/arm/kernel/kprobes.c | 10 +-
arch/arm/kernel/kprobes.h | 8 +-
arch/arm/kernel/probes-arm.c | 32 +++-
arch/arm/kernel/probes-arm.h | 15 +-
arch/arm/kernel/probes-thumb.c | 152 ++++++++++++++++---
arch/arm/kernel/probes-thumb.h | 31 +++-
arch/arm/kernel/probes.c | 76 +++++++++-
arch/arm/kernel/probes.h | 27 +++-
arch/arm/kernel/uprobes-arm.c | 12 +-
arch/arm/kernel/uprobes.h | 2 +-
kernel/kprobes.c | 6 +
20 files changed, 679 insertions(+), 74 deletions(-)
create mode 100644 arch/arm/kernel/kprobes-opt-arm.c
--
1.8.4
Copy old kprobe to newly alloced optimized_kprobe before
arch_prepare_optimized_kprobe(). Original kprove can brings more
information to optimizer.
v1 -> v2:
- Bugfix: copy p->addr when alloc_aggr_kprobe.
Signed-off-by: Wang Nan <[email protected]>
---
kernel/kprobes.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 3995f54..e9868ec 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -730,7 +730,13 @@ static struct kprobe *alloc_aggr_kprobe(struct kprobe *p)
return NULL;
INIT_LIST_HEAD(&op->list);
+
+ /*
+ * copy gives arch_prepare_optimized_kprobe
+ * more information
+ */
op->kp.addr = p->addr;
+ copy_kprobe(p, &op->kp);
arch_prepare_optimized_kprobe(op);
return &op->kp;
--
1.8.4
This patch use previous introduced checker on store instructions,
record stack consumption informations to arch_probes_insn. With such
information, kprobe opt can decide how much stack needs to be
protected.
Signed-off-by: Wang Nan <[email protected]>
---
arch/arm/include/asm/probes.h | 1 +
arch/arm/kernel/kprobes-arm.c | 8 ++---
arch/arm/kernel/kprobes-thumb.c | 8 ++---
arch/arm/kernel/probes-arm.c | 19 +++++++++++
arch/arm/kernel/probes-arm.h | 7 +++++
arch/arm/kernel/probes-thumb.c | 70 +++++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/probes-thumb.h | 9 ++++++
arch/arm/kernel/probes.c | 64 +++++++++++++++++++++++++++++++++++++
arch/arm/kernel/probes.h | 13 ++++++++
arch/arm/kernel/uprobes-arm.c | 8 ++---
10 files changed, 195 insertions(+), 12 deletions(-)
diff --git a/arch/arm/include/asm/probes.h b/arch/arm/include/asm/probes.h
index 806cfe6..ccf9af3 100644
--- a/arch/arm/include/asm/probes.h
+++ b/arch/arm/include/asm/probes.h
@@ -38,6 +38,7 @@ struct arch_probes_insn {
probes_check_cc *insn_check_cc;
probes_insn_singlestep_t *insn_singlestep;
probes_insn_fn_t *insn_fn;
+ int stack_space;
};
#endif
diff --git a/arch/arm/kernel/kprobes-arm.c b/arch/arm/kernel/kprobes-arm.c
index 1094ff1..bb1bec3 100644
--- a/arch/arm/kernel/kprobes-arm.c
+++ b/arch/arm/kernel/kprobes-arm.c
@@ -316,11 +316,11 @@ const struct decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
[PROBES_MUL2] = {.handler = emulate_rd16rn12rm0rs8_rwflags_nopc},
[PROBES_SWP] = {.handler = emulate_rd12rn16rm0_rwflags_nopc},
[PROBES_LDRD] = {.handler = emulate_ldrdstrd},
- [PROBES_STRD] = {.handler = emulate_ldrdstrd},
+ [PROBES_STRD] = {.checker = probes_check_arm_store_extra, .handler = emulate_ldrdstrd},
[PROBES_LOAD_EXTRA] = {.handler = emulate_ldr},
[PROBES_LOAD] = {.handler = emulate_ldr},
- [PROBES_STORE_EXTRA] = {.handler = emulate_str},
- [PROBES_STORE] = {.handler = emulate_str},
+ [PROBES_STORE_EXTRA] = {.checker = probes_check_arm_store_extra, .handler = emulate_str},
+ [PROBES_STORE] = {.checker = probes_check_arm_store, .handler = emulate_str},
[PROBES_MOV_IP_SP] = {.handler = simulate_mov_ipsp},
[PROBES_DATA_PROCESSING_REG] = {
.handler = emulate_rd12rn16rm0rs8_rwflags},
@@ -341,5 +341,5 @@ const struct decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
[PROBES_BITFIELD] = {.handler = emulate_rd12rm0_noflags_nopc},
[PROBES_BRANCH] = {.handler = simulate_bbl},
[PROBES_LDM] = {.decoder = kprobe_decode_ldmstm},
- [PROBES_STM] = {.decoder = kprobe_decode_ldmstm},
+ [PROBES_STM] = {.checker = probes_check_stm, .decoder = kprobe_decode_ldmstm},
};
diff --git a/arch/arm/kernel/kprobes-thumb.c b/arch/arm/kernel/kprobes-thumb.c
index c6426b6..5da4231 100644
--- a/arch/arm/kernel/kprobes-thumb.c
+++ b/arch/arm/kernel/kprobes-thumb.c
@@ -615,7 +615,7 @@ const struct decode_action kprobes_t16_actions[NUM_PROBES_T16_ACTIONS] = {
[PROBES_T16_ADD_SP] = {.handler = t16_simulate_add_sp_imm},
[PROBES_T16_CBZ] = {.handler = t16_simulate_cbz},
[PROBES_T16_SIGN_EXTEND] = {.handler = t16_emulate_loregs_rwflags},
- [PROBES_T16_PUSH] = {.decoder = t16_decode_push},
+ [PROBES_T16_PUSH] = {.checker = t16_check_push, .decoder = t16_decode_push},
[PROBES_T16_POP] = {.decoder = t16_decode_pop},
[PROBES_T16_SEV] = {.handler = probes_emulate_none},
[PROBES_T16_WFE] = {.handler = probes_simulate_nop},
@@ -639,9 +639,9 @@ const struct decode_action kprobes_t16_actions[NUM_PROBES_T16_ACTIONS] = {
const struct decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
[PROBES_T32_LDM] = {.decoder = t32_decode_ldmstm},
- [PROBES_T32_STM] = {.decoder = t32_decode_ldmstm},
+ [PROBES_T32_STM] = {.checker = probes_check_stm, .decoder = t32_decode_ldmstm},
[PROBES_T32_LDRD] = {.handler = t32_emulate_ldrdstrd},
- [PROBES_T32_STRD] = {.handler = t32_emulate_ldrdstrd},
+ [PROBES_T32_STRD] = {.checker = t32_check_strd, .handler = t32_emulate_ldrdstrd},
[PROBES_T32_TABLE_BRANCH] = {.handler = t32_simulate_table_branch},
[PROBES_T32_TST] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
[PROBES_T32_MOV] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
@@ -661,7 +661,7 @@ const struct decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
[PROBES_T32_PLDI] = {.handler = probes_simulate_nop},
[PROBES_T32_LDR_LIT] = {.handler = t32_simulate_ldr_literal},
[PROBES_T32_LDR] = {.handler = t32_emulate_ldrstr},
- [PROBES_T32_STR] = {.handler = t32_emulate_ldrstr},
+ [PROBES_T32_STR] = {.checker = t32_check_str, .handler = t32_emulate_ldrstr},
[PROBES_T32_SIGN_EXTEND] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
[PROBES_T32_MEDIA] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
[PROBES_T32_REVERSE] = {.handler = t32_emulate_rd8rn16_noflags},
diff --git a/arch/arm/kernel/probes-arm.c b/arch/arm/kernel/probes-arm.c
index 148153e..90d2f29 100644
--- a/arch/arm/kernel/probes-arm.c
+++ b/arch/arm/kernel/probes-arm.c
@@ -109,6 +109,25 @@ void __kprobes simulate_mov_ipsp(probes_opcode_t insn,
regs->uregs[12] = regs->uregs[13];
}
+enum probes_insn __kprobes probes_check_arm_store(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ int imm = insn & 0xfff;
+ check_insn_stack_regs(insn, asi, h, imm);
+ return INSN_GOOD;
+}
+
+enum probes_insn __kprobes probes_check_arm_store_extra(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ int imm = ((insn & 0xf00) >> 4) + (insn & 0xf);
+ check_insn_stack_regs(insn, asi, h, imm);
+ return INSN_GOOD;
+}
+
+
/*
* For the instruction masking and comparisons in all the "space_*"
* functions below, Do _not_ rearrange the order of tests unless
diff --git a/arch/arm/kernel/probes-arm.h b/arch/arm/kernel/probes-arm.h
index 18ffc9a..d0ad9a4 100644
--- a/arch/arm/kernel/probes-arm.h
+++ b/arch/arm/kernel/probes-arm.h
@@ -66,6 +66,13 @@ void __kprobes simulate_mrs(probes_opcode_t opcode,
void __kprobes simulate_mov_ipsp(probes_opcode_t opcode,
struct arch_probes_insn *asi, struct pt_regs *regs);
+enum probes_insn __kprobes probes_check_arm_store(probes_opcode_t,
+ struct arch_probes_insn *,
+ const struct decode_header *);
+enum probes_insn __kprobes probes_check_arm_store_extra(probes_opcode_t,
+ struct arch_probes_insn *,
+ const struct decode_header *);
+
extern const union decode_item probes_decode_arm_table[];
enum probes_insn arm_probes_decode_insn(probes_opcode_t,
diff --git a/arch/arm/kernel/probes-thumb.c b/arch/arm/kernel/probes-thumb.c
index 749d4cd..5d0c936 100644
--- a/arch/arm/kernel/probes-thumb.c
+++ b/arch/arm/kernel/probes-thumb.c
@@ -15,6 +15,76 @@
#include "probes.h"
#include "probes-thumb.h"
+enum probes_insn __kprobes t32_check_strd(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ int imm = insn & 0xff;
+ check_insn_stack_regs(insn, asi, h, imm);
+ return INSN_GOOD;
+}
+
+/*
+ * Note: This function doesn't process PROBES_T32_STRD.
+ */
+enum probes_insn __kprobes t32_check_str(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ int rn = -1, rm = -1;
+ u32 regs = h->type_regs.bits >> DECODE_TYPE_BITS;
+ int index, add;
+
+ /* Rn is used in every cases */
+ BUG_ON((regs & 0xf0000) == 0);
+ rn = (insn & 0xf0000) >> 16;
+ if ((regs & 0xf) != 0)
+ rm = insn & 0xf;
+
+ /*
+ * Rn is not SP. Rm can't be sp in any case.
+ * So it is not a stack store.
+ */
+ if (rn != 0xd)
+ return INSN_GOOD;
+
+ /*
+ * For 'str? rx, [sp, ry]', ry can be negative. In addition,
+ * index is true in every cases, so unable to determine stack
+ * consumption.
+ */
+ if (rm != -1) {
+ asi->stack_space = -1;
+ return INSN_GOOD;
+ }
+
+ /*
+ * For 'str? rx, [sp, #+/-<imm>]', if bit 23 is set, index
+ * and add are both set. Else, index and add are determined
+ * by P bit and U bit (bit 10, 9)
+ */
+ if (insn & 0x800000)
+ index = add = 1;
+ else {
+ index = (insn & (1 << 10));
+ add = (insn &(1 << 9));
+ }
+
+ if (!index || add)
+ return INSN_GOOD;
+
+ asi->stack_space = insn & 0xff;
+ return INSN_GOOD;
+}
+
+enum probes_insn __kprobes t16_check_push(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ unsigned int reglist = insn & 0x1ff;
+ asi->stack_space = hweight32(reglist) * 4;
+ return INSN_GOOD;
+}
static const union decode_item t32_table_1110_100x_x0xx[] = {
/* Load/store multiple instructions */
diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
index a9c65c9..f908b12 100644
--- a/arch/arm/kernel/probes-thumb.h
+++ b/arch/arm/kernel/probes-thumb.h
@@ -100,4 +100,13 @@ enum probes_insn __kprobes
thumb32_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
bool emulate, const struct decode_action *actions);
+enum probes_insn __kprobes t32_check_strd(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h);
+enum probes_insn __kprobes t32_check_str(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h);
+enum probes_insn __kprobes t16_check_push(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h);
#endif
diff --git a/arch/arm/kernel/probes.c b/arch/arm/kernel/probes.c
index 6164b4d..8428fe8 100644
--- a/arch/arm/kernel/probes.c
+++ b/arch/arm/kernel/probes.c
@@ -188,6 +188,25 @@ void __kprobes probes_emulate_none(probes_opcode_t opcode,
asi->insn_fn();
}
+/* ARM and Thumb can share this checker */
+enum probes_insn __kprobes probes_check_stm(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h)
+{
+ unsigned int reglist = insn & 0xffff;
+ int ubit = insn & (1 << 23);
+ int pbit = insn & (1 << 24);
+ int rn = (insn >> 16) & 0xf;
+
+ /* This is stmi?, doesn't require extra stack */
+ if (ubit)
+ return INSN_GOOD;
+ /* If pbit == ubit (== 0), this is stmda, one dword is saved */
+ asi->stack_space = (rn == 0xd) ?
+ (hweight32(reglist) - ((!pbit == !ubit) ? 1 : 0)) * 4 : 0;
+ return INSN_GOOD;
+}
+
/*
* Prepare an instruction slot to receive an instruction for emulating.
* This is done by placing a subroutine return after the location where the
@@ -395,6 +414,8 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
bool matched = false;
probes_opcode_t origin_insn = insn;
+ asi->stack_space = 0;
+
if (emulate)
insn = prepare_emulated_insn(insn, asi, thumb);
@@ -464,3 +485,46 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
}
}
}
+
+int __kprobes check_insn_stack_regs(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h,
+ int imm)
+{
+ u32 regs = h->type_regs.bits >> DECODE_TYPE_BITS;
+ int rn = -1, rm = -1, index, add;
+ asi->stack_space = 0;
+
+ if (((regs >> 16) & 0xf) != REG_TYPE_NONE)
+ rn = (insn >> 16) & 0xf;
+
+ if ((regs & 0xf) != REG_TYPE_NONE)
+ rm = insn & 0xf;
+
+ if ((rn != 13) && (rm != 13))
+ return NOT_STACK_STORE;
+
+ index = insn & (1 << 24);
+ add = insn & (1 << 23);
+
+ if (!index)
+ return NOT_STACK_STORE;
+
+ /*
+ * Even if insn is 'str r0, [sp], +<Rm>', Rm may less than 0.
+ * Therefore if both Rn and Rm are registers and !index,
+ * We are unable to determine whether it is a stack store.
+ */
+ if ((rn != -1) && (rm != -1)) {
+ asi->stack_space = -1;
+ return STACK_REG;
+ }
+
+ /* 'str(d/h) r0, [sp], #+/-<imm>' */
+ /* or 'str(d/h) r0, [sp, #+<imm>'] */
+ if (add)
+ return NOT_STACK_STORE;
+
+ asi->stack_space = imm;
+ return STACK_IMM;
+}
diff --git a/arch/arm/kernel/probes.h b/arch/arm/kernel/probes.h
index c56dd3d..4c0a04a 100644
--- a/arch/arm/kernel/probes.h
+++ b/arch/arm/kernel/probes.h
@@ -410,4 +410,17 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
const union decode_item *table, bool thumb, bool emulate,
const struct decode_action *actions);
+enum probes_insn __kprobes probes_check_stm(probes_opcode_t,
+ struct arch_probes_insn *,
+ const struct decode_header *);
+
+enum {
+ NOT_STACK_STORE,
+ STACK_REG,
+ STACK_IMM,
+};
+int __kprobes check_insn_stack_regs(probes_opcode_t insn,
+ struct arch_probes_insn *asi,
+ const struct decode_header *h,
+ int imm);
#endif
diff --git a/arch/arm/kernel/uprobes-arm.c b/arch/arm/kernel/uprobes-arm.c
index 0a8caa3..1da524d 100644
--- a/arch/arm/kernel/uprobes-arm.c
+++ b/arch/arm/kernel/uprobes-arm.c
@@ -208,11 +208,11 @@ const struct decode_action uprobes_probes_actions[] = {
[PROBES_MUL2] = {.handler = probes_simulate_nop},
[PROBES_SWP] = {.handler = probes_simulate_nop},
[PROBES_LDRD] = {.decoder = decode_pc_ro},
- [PROBES_STRD] = {.decoder = decode_pc_ro},
+ [PROBES_STRD] = {.checker = probes_check_arm_store_extra, .decoder = decode_pc_ro},
[PROBES_LOAD_EXTRA] = {.decoder = decode_pc_ro},
[PROBES_LOAD] = {.decoder = decode_ldr},
- [PROBES_STORE_EXTRA] = {.decoder = decode_pc_ro},
- [PROBES_STORE] = {.decoder = decode_pc_ro},
+ [PROBES_STORE_EXTRA] = {.checker = probes_check_arm_store_extra, .decoder = decode_pc_ro},
+ [PROBES_STORE] = {.checker = probes_check_arm_store, .decoder = decode_pc_ro},
[PROBES_MOV_IP_SP] = {.handler = simulate_mov_ipsp},
[PROBES_DATA_PROCESSING_REG] = {
.decoder = decode_rd12rn16rm0rs8_rwflags},
@@ -232,5 +232,5 @@ const struct decode_action uprobes_probes_actions[] = {
[PROBES_BITFIELD] = {.handler = probes_simulate_nop},
[PROBES_BRANCH] = {.handler = simulate_bbl},
[PROBES_LDM] = {.decoder = uprobe_decode_ldmstm},
- [PROBES_STM] = {.decoder = uprobe_decode_ldmstm}
+ [PROBES_STM] = {.checker = probes_check_stm, .decoder = uprobe_decode_ldmstm}
};
--
1.8.4
This patch seprates actions for load and store. Following patches will
check store instructions for more informations.
Coverage test complains register test coverage missing after this
sepration. This patch introduces one testcase for it.
Signed-off-by: Wang Nan <[email protected]>
---
arch/arm/kernel/kprobes-arm.c | 6 ++-
arch/arm/kernel/kprobes-test-arm.c | 1 +
arch/arm/kernel/kprobes-test-thumb.c | 13 ++++++
arch/arm/kernel/kprobes-thumb.c | 18 ++++++---
arch/arm/kernel/probes-arm.c | 11 +++--
arch/arm/kernel/probes-arm.h | 6 ++-
arch/arm/kernel/probes-thumb.c | 78 +++++++++++++++++++++++++++---------
arch/arm/kernel/probes-thumb.h | 18 ++++++---
arch/arm/kernel/uprobes-arm.c | 6 ++-
9 files changed, 116 insertions(+), 41 deletions(-)
diff --git a/arch/arm/kernel/kprobes-arm.c b/arch/arm/kernel/kprobes-arm.c
index 6df9f1f..1094ff1 100644
--- a/arch/arm/kernel/kprobes-arm.c
+++ b/arch/arm/kernel/kprobes-arm.c
@@ -315,7 +315,8 @@ const struct decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
[PROBES_MUL1] = {.handler = emulate_rdlo12rdhi16rn0rm8_rwflags_nopc},
[PROBES_MUL2] = {.handler = emulate_rd16rn12rm0rs8_rwflags_nopc},
[PROBES_SWP] = {.handler = emulate_rd12rn16rm0_rwflags_nopc},
- [PROBES_LDRSTRD] = {.handler = emulate_ldrdstrd},
+ [PROBES_LDRD] = {.handler = emulate_ldrdstrd},
+ [PROBES_STRD] = {.handler = emulate_ldrdstrd},
[PROBES_LOAD_EXTRA] = {.handler = emulate_ldr},
[PROBES_LOAD] = {.handler = emulate_ldr},
[PROBES_STORE_EXTRA] = {.handler = emulate_str},
@@ -339,5 +340,6 @@ const struct decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
[PROBES_MUL_ADD] = {.handler = emulate_rd16rn12rm0rs8_rwflags_nopc},
[PROBES_BITFIELD] = {.handler = emulate_rd12rm0_noflags_nopc},
[PROBES_BRANCH] = {.handler = simulate_bbl},
- [PROBES_LDMSTM] = {.decoder = kprobe_decode_ldmstm}
+ [PROBES_LDM] = {.decoder = kprobe_decode_ldmstm},
+ [PROBES_STM] = {.decoder = kprobe_decode_ldmstm},
};
diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
index cb14242..264c064 100644
--- a/arch/arm/kernel/kprobes-test-arm.c
+++ b/arch/arm/kernel/kprobes-test-arm.c
@@ -571,6 +571,7 @@ void kprobe_arm_test_cases(void)
TEST_RPR( "strd r",2, VAL1,", [r",5, 24,"], r",4,48,"")
TEST_RPR( "strd r",10,VAL2,", [r",9, 48,"], -r",7,24,"")
TEST_UNSUPPORTED(__inst_arm(0xe1afc0fa) " @ strd r12, [pc, r10]!")
+ TEST_UNSUPPORTED(__inst_arm(0xe1aac0ff) " @ strd r12, [r10, pc]!")
TEST_PR( "ldrd r0, [r",0, 48,", -r",2,24,"]")
TEST_PR( "ldrmid r8, [r",13,0, ", r",12,48,"]")
diff --git a/arch/arm/kernel/kprobes-test-thumb.c b/arch/arm/kernel/kprobes-test-thumb.c
index 844dd10..ed863c4 100644
--- a/arch/arm/kernel/kprobes-test-thumb.c
+++ b/arch/arm/kernel/kprobes-test-thumb.c
@@ -410,6 +410,13 @@ void kprobe_thumb32_test_cases(void)
TEST_UNSUPPORTED(__inst_thumb32(0xe9d47d00) " @ ldrd r7, sp, [r4]")
TEST_UNSUPPORTED(__inst_thumb32(0xe9d47f00) " @ ldrd r7, pc, [r4]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9efec04) " @ strd r14, r12, [pc, #16]!")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe8efec04) " @ strd r14, r12, [pc], #16")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9c4d800) " @ strd sp, r8, [r4]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9c4f800) " @ strd pc, r8, [r4]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9c47d00) " @ strd r7, sp, [r4]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xe9c47f00) " @ strd r7, pc, [r4]")
+
TEST_RRP("strd r",0, VAL1,", r",1, VAL2,", [r",1, 24,", #-16]")
TEST_RR( "strd r",12,VAL2,", r",14,VAL1,", [sp, #16]")
TEST_RRP("strd r",1, VAL1,", r",0, VAL2,", [r",7, 24,", #-16]!")
@@ -832,6 +839,12 @@ CONDITION_INSTRUCTIONS(22,
TEST("str sp, [sp]")
TEST_UNSUPPORTED(__inst_thumb32(0xf8cfe000) " @ str r14, [pc]")
TEST_UNSUPPORTED(__inst_thumb32(0xf8cef000) " @ str pc, [r14]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf841100f) " @ str r1, [r1, pc]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf841100d) " @ str r1, [r1, sp]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf8a1d000) " @ strh sp, [r1]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf821d002) " @ strh sp, [r1, r2]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf822100f) " @ strh r1, [r2, pc]")
+ TEST_UNSUPPORTED(__inst_thumb32(0xf822100d) " @ strh r1, [r2, sp]")
TEST_GROUP("Advanced SIMD element or structure load/store instructions")
diff --git a/arch/arm/kernel/kprobes-thumb.c b/arch/arm/kernel/kprobes-thumb.c
index 0672457..c6426b6 100644
--- a/arch/arm/kernel/kprobes-thumb.c
+++ b/arch/arm/kernel/kprobes-thumb.c
@@ -626,17 +626,22 @@ const struct decode_action kprobes_t16_actions[NUM_PROBES_T16_ACTIONS] = {
[PROBES_T16_LDR_LIT] = {.handler = t16_simulate_ldr_literal},
[PROBES_T16_BLX] = {.handler = t16_simulate_bxblx},
[PROBES_T16_HIREGOPS] = {.decoder = t16_decode_hiregs},
- [PROBES_T16_LDRHSTRH] = {.handler = t16_emulate_loregs_rwflags},
- [PROBES_T16_LDRSTR] = {.handler = t16_simulate_ldrstr_sp_relative},
+ [PROBES_T16_LDRH] = {.handler = t16_emulate_loregs_rwflags},
+ [PROBES_T16_STRH] = {.handler = t16_emulate_loregs_rwflags},
+ [PROBES_T16_LDR] = {.handler = t16_simulate_ldrstr_sp_relative},
+ [PROBES_T16_STR] = {.handler = t16_simulate_ldrstr_sp_relative},
[PROBES_T16_ADR] = {.handler = t16_simulate_reladr},
- [PROBES_T16_LDMSTM] = {.handler = t16_emulate_loregs_rwflags},
+ [PROBES_T16_LDM] = {.handler = t16_emulate_loregs_rwflags},
+ [PROBES_T16_STM] = {.handler = t16_emulate_loregs_rwflags},
[PROBES_T16_BRANCH_COND] = {.decoder = t16_decode_cond_branch},
[PROBES_T16_BRANCH] = {.handler = t16_simulate_branch},
};
const struct decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
- [PROBES_T32_LDMSTM] = {.decoder = t32_decode_ldmstm},
- [PROBES_T32_LDRDSTRD] = {.handler = t32_emulate_ldrdstrd},
+ [PROBES_T32_LDM] = {.decoder = t32_decode_ldmstm},
+ [PROBES_T32_STM] = {.decoder = t32_decode_ldmstm},
+ [PROBES_T32_LDRD] = {.handler = t32_emulate_ldrdstrd},
+ [PROBES_T32_STRD] = {.handler = t32_emulate_ldrdstrd},
[PROBES_T32_TABLE_BRANCH] = {.handler = t32_simulate_table_branch},
[PROBES_T32_TST] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
[PROBES_T32_MOV] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
@@ -655,7 +660,8 @@ const struct decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
[PROBES_T32_BRANCH] = {.handler = t32_simulate_branch},
[PROBES_T32_PLDI] = {.handler = probes_simulate_nop},
[PROBES_T32_LDR_LIT] = {.handler = t32_simulate_ldr_literal},
- [PROBES_T32_LDRSTR] = {.handler = t32_emulate_ldrstr},
+ [PROBES_T32_LDR] = {.handler = t32_emulate_ldrstr},
+ [PROBES_T32_STR] = {.handler = t32_emulate_ldrstr},
[PROBES_T32_SIGN_EXTEND] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
[PROBES_T32_MEDIA] = {.handler = t32_emulate_rd8rn16rm0_rwflags},
[PROBES_T32_REVERSE] = {.handler = t32_emulate_rd8rn16_noflags},
diff --git a/arch/arm/kernel/probes-arm.c b/arch/arm/kernel/probes-arm.c
index 3d00aa7..148153e 100644
--- a/arch/arm/kernel/probes-arm.c
+++ b/arch/arm/kernel/probes-arm.c
@@ -270,13 +270,17 @@ static const union decode_item arm_cccc_000x_____1xx1_table[] = {
DECODE_REJECT (0x0e10e0d0, 0x0000e0d0),
/* LDRD (register) cccc 000x x0x0 xxxx xxxx xxxx 1101 xxxx */
+ DECODE_EMULATEX (0x0e5000f0, 0x000000d0, PROBES_LDRD,
+ REGS(NOPCWB, NOPCX, 0, 0, NOPC)),
/* STRD (register) cccc 000x x0x0 xxxx xxxx xxxx 1111 xxxx */
- DECODE_EMULATEX (0x0e5000d0, 0x000000d0, PROBES_LDRSTRD,
+ DECODE_EMULATEX (0x0e5000f0, 0x000000f0, PROBES_STRD,
REGS(NOPCWB, NOPCX, 0, 0, NOPC)),
/* LDRD (immediate) cccc 000x x1x0 xxxx xxxx xxxx 1101 xxxx */
+ DECODE_EMULATEX (0x0e5000f0, 0x004000d0, PROBES_LDRD,
+ REGS(NOPCWB, NOPCX, 0, 0, 0)),
/* STRD (immediate) cccc 000x x1x0 xxxx xxxx xxxx 1111 xxxx */
- DECODE_EMULATEX (0x0e5000d0, 0x004000d0, PROBES_LDRSTRD,
+ DECODE_EMULATEX (0x0e5000f0, 0x004000f0, PROBES_STRD,
REGS(NOPCWB, NOPCX, 0, 0, 0)),
/* STRH (register) cccc 000x x0x0 xxxx xxxx xxxx 1011 xxxx */
@@ -601,8 +605,9 @@ static const union decode_item arm_cccc_100x_table[] = {
/* Block data transfer instructions */
/* LDM cccc 100x x0x1 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_CUSTOM (0x0e500000, 0x08100000, PROBES_LDM),
/* STM cccc 100x x0x0 xxxx xxxx xxxx xxxx xxxx */
- DECODE_CUSTOM (0x0e400000, 0x08000000, PROBES_LDMSTM),
+ DECODE_CUSTOM (0x0e500000, 0x08000000, PROBES_STM),
/* STM (user registers) cccc 100x x1x0 xxxx xxxx xxxx xxxx xxxx */
/* LDM (user registers) cccc 100x x1x1 xxxx 0xxx xxxx xxxx xxxx */
diff --git a/arch/arm/kernel/probes-arm.h b/arch/arm/kernel/probes-arm.h
index 6ecc25a..18ffc9a 100644
--- a/arch/arm/kernel/probes-arm.h
+++ b/arch/arm/kernel/probes-arm.h
@@ -28,7 +28,8 @@ enum probes_arm_action {
PROBES_MUL1,
PROBES_MUL2,
PROBES_SWP,
- PROBES_LDRSTRD,
+ PROBES_LDRD,
+ PROBES_STRD,
PROBES_LOAD,
PROBES_STORE,
PROBES_LOAD_EXTRA,
@@ -49,7 +50,8 @@ enum probes_arm_action {
PROBES_MUL_ADD,
PROBES_BITFIELD,
PROBES_BRANCH,
- PROBES_LDMSTM,
+ PROBES_LDM,
+ PROBES_STM,
NUM_PROBES_ARM_ACTIONS
};
diff --git a/arch/arm/kernel/probes-thumb.c b/arch/arm/kernel/probes-thumb.c
index 72aa217..749d4cd 100644
--- a/arch/arm/kernel/probes-thumb.c
+++ b/arch/arm/kernel/probes-thumb.c
@@ -37,10 +37,11 @@ static const union decode_item t32_table_1110_100x_x0xx[] = {
DECODE_REJECT (0xfe402000, 0xe8002000),
/* STMIA 1110 1000 10x0 xxxx xxxx xxxx xxxx xxxx */
- /* LDMIA 1110 1000 10x1 xxxx xxxx xxxx xxxx xxxx */
/* STMDB 1110 1001 00x0 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_CUSTOM (0xfe500000, 0xe8000000, PROBES_T32_STM),
+ /* LDMIA 1110 1000 10x1 xxxx xxxx xxxx xxxx xxxx */
/* LDMDB 1110 1001 00x1 xxxx xxxx xxxx xxxx xxxx */
- DECODE_CUSTOM (0xfe400000, 0xe8000000, PROBES_T32_LDMSTM),
+ DECODE_CUSTOM (0xfe500000, 0xe8100000, PROBES_T32_LDM),
DECODE_END
};
@@ -49,11 +50,15 @@ static const union decode_item t32_table_1110_100x_x1xx[] = {
/* Load/store dual, load/store exclusive, table branch */
/* STRD (immediate) 1110 1000 x110 xxxx xxxx xxxx xxxx xxxx */
- /* LDRD (immediate) 1110 1000 x111 xxxx xxxx xxxx xxxx xxxx */
- DECODE_OR (0xff600000, 0xe8600000),
+ DECODE_OR (0xff700000, 0xe8600000),
/* STRD (immediate) 1110 1001 x1x0 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_EMULATEX (0xff500000, 0xe9400000, PROBES_T32_STRD,
+ REGS(NOPCWB, NOSPPC, NOSPPC, 0, 0)),
+
+ /* LDRD (immediate) 1110 1000 x111 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_OR (0xff700000, 0xe8700000),
/* LDRD (immediate) 1110 1001 x1x1 xxxx xxxx xxxx xxxx xxxx */
- DECODE_EMULATEX (0xff400000, 0xe9400000, PROBES_T32_LDRDSTRD,
+ DECODE_EMULATEX (0xff500000, 0xe9500000, PROBES_T32_LDRD,
REGS(NOPCWB, NOSPPC, NOSPPC, 0, 0)),
/* TBB 1110 1000 1101 xxxx xxxx xxxx 0000 xxxx */
@@ -340,16 +345,29 @@ static const union decode_item t32_table_1111_100x[] = {
REGS(PC, ANY, 0, 0, 0)),
/* STR (immediate) 1111 1000 0100 xxxx xxxx 1xxx xxxx xxxx */
- /* LDR (immediate) 1111 1000 0101 xxxx xxxx 1xxx xxxx xxxx */
- DECODE_OR (0xffe00800, 0xf8400800),
+ DECODE_OR (0xfff00800, 0xf8400800),
/* STR (immediate) 1111 1000 1100 xxxx xxxx xxxx xxxx xxxx */
+ /*
+ * Reject PC for Rt. PC has already rejected by
+ * 0xff1f0000, 0xf80f0000 and 0xff10f000, 0xf800f000.
+ * Suppress complain on coverage in test code.
+ */
+ DECODE_EMULATEX (0xfff00000, 0xf8c00000, PROBES_T32_STR,
+ REGS(NOPCX, NOPCX, 0, 0, 0)),
+ /* LDR (immediate) 1111 1000 0101 xxxx xxxx 1xxx xxxx xxxx */
+ DECODE_OR (0xfff00800, 0xf8500800),
/* LDR (immediate) 1111 1000 1101 xxxx xxxx xxxx xxxx xxxx */
- DECODE_EMULATEX (0xffe00000, 0xf8c00000, PROBES_T32_LDRSTR,
+ DECODE_EMULATEX (0xfff00000, 0xf8d00000, PROBES_T32_LDR,
REGS(NOPCX, ANY, 0, 0, 0)),
-
/* STR (register) 1111 1000 0100 xxxx xxxx 0000 00xx xxxx */
+ /*
+ * Rt == PC and Rn == PC have already been rejected by
+ * 0xff1f0000, 0xf80f0000 and 0xff10f000, 0xf800f000
+ */
+ DECODE_EMULATEX (0xfff00fc0, 0xf8400000, PROBES_T32_STR,
+ REGS(NOPCX, NOPCX, 0, 0, NOSPPC)),
/* LDR (register) 1111 1000 0101 xxxx xxxx 0000 00xx xxxx */
- DECODE_EMULATEX (0xffe00fc0, 0xf8400000, PROBES_T32_LDRSTR,
+ DECODE_EMULATEX (0xfff00fc0, 0xf8500000, PROBES_T32_LDR,
REGS(NOPCX, ANY, 0, 0, NOSPPC)),
/* LDRB (literal) 1111 1000 x001 1111 xxxx xxxx xxxx xxxx */
@@ -361,27 +379,35 @@ static const union decode_item t32_table_1111_100x[] = {
/* STRB (immediate) 1111 1000 0000 xxxx xxxx 1xxx xxxx xxxx */
/* STRH (immediate) 1111 1000 0010 xxxx xxxx 1xxx xxxx xxxx */
+ DECODE_OR (0xffd00800, 0xf8000800),
+ /* STRB (immediate) 1111 1000 1000 xxxx xxxx xxxx xxxx xxxx */
+ /* STRH (immediate) 1111 1000 1010 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_EMULATEX (0xffd00000, 0xf8800000, PROBES_T32_STR,
+ REGS(NOPCX, NOSPPCX, 0, 0, 0)),
+
/* LDRB (immediate) 1111 1000 0001 xxxx xxxx 1xxx xxxx xxxx */
/* LDRSB (immediate) 1111 1001 0001 xxxx xxxx 1xxx xxxx xxxx */
/* LDRH (immediate) 1111 1000 0011 xxxx xxxx 1xxx xxxx xxxx */
/* LDRSH (immediate) 1111 1001 0011 xxxx xxxx 1xxx xxxx xxxx */
- DECODE_OR (0xfec00800, 0xf8000800),
- /* STRB (immediate) 1111 1000 1000 xxxx xxxx xxxx xxxx xxxx */
- /* STRH (immediate) 1111 1000 1010 xxxx xxxx xxxx xxxx xxxx */
+ DECODE_OR (0xfed00800, 0xf8100800),
+
/* LDRB (immediate) 1111 1000 1001 xxxx xxxx xxxx xxxx xxxx */
/* LDRSB (immediate) 1111 1001 1001 xxxx xxxx xxxx xxxx xxxx */
/* LDRH (immediate) 1111 1000 1011 xxxx xxxx xxxx xxxx xxxx */
/* LDRSH (immediate) 1111 1001 1011 xxxx xxxx xxxx xxxx xxxx */
- DECODE_EMULATEX (0xfec00000, 0xf8800000, PROBES_T32_LDRSTR,
+ DECODE_EMULATEX (0xfed00000, 0xf8900000, PROBES_T32_LDR,
REGS(NOPCX, NOSPPCX, 0, 0, 0)),
/* STRB (register) 1111 1000 0000 xxxx xxxx 0000 00xx xxxx */
/* STRH (register) 1111 1000 0010 xxxx xxxx 0000 00xx xxxx */
+ DECODE_EMULATEX (0xffd00fc0, 0xf8000000, PROBES_T32_STR,
+ REGS(NOPCX, NOSPPCX, 0, 0, NOSPPC)),
+
/* LDRB (register) 1111 1000 0001 xxxx xxxx 0000 00xx xxxx */
/* LDRSB (register) 1111 1001 0001 xxxx xxxx 0000 00xx xxxx */
/* LDRH (register) 1111 1000 0011 xxxx xxxx 0000 00xx xxxx */
/* LDRSH (register) 1111 1001 0011 xxxx xxxx 0000 00xx xxxx */
- DECODE_EMULATEX (0xfe800fc0, 0xf8000000, PROBES_T32_LDRSTR,
+ DECODE_EMULATEX (0xfed00fc0, 0xf8100000, PROBES_T32_LDR,
REGS(NOPCX, NOSPPCX, 0, 0, NOSPPC)),
/* Other unallocated instructions... */
@@ -778,23 +804,34 @@ const union decode_item probes_decode_thumb16_table[] = {
/* STR (register) 0101 000x xxxx xxxx */
/* STRH (register) 0101 001x xxxx xxxx */
+ DECODE_EMULATE (0xfc00, 0x5000, PROBES_T16_STRH),
/* STRB (register) 0101 010x xxxx xxxx */
+ DECODE_EMULATE (0xfe00, 0x5400, PROBES_T16_STRH),
/* LDRSB (register) 0101 011x xxxx xxxx */
+ DECODE_EMULATE (0xfe00, 0x5600, PROBES_T16_LDRH),
+
/* LDR (register) 0101 100x xxxx xxxx */
/* LDRH (register) 0101 101x xxxx xxxx */
/* LDRB (register) 0101 110x xxxx xxxx */
/* LDRSH (register) 0101 111x xxxx xxxx */
+ DECODE_EMULATE (0xf800, 0x5800, PROBES_T16_LDRH),
+
/* STR (immediate, Thumb) 0110 0xxx xxxx xxxx */
- /* LDR (immediate, Thumb) 0110 1xxx xxxx xxxx */
/* STRB (immediate, Thumb) 0111 0xxx xxxx xxxx */
+ DECODE_EMULATE (0xe800, 0x6000, PROBES_T16_STRH),
+
+ /* LDR (immediate, Thumb) 0110 1xxx xxxx xxxx */
/* LDRB (immediate, Thumb) 0111 1xxx xxxx xxxx */
- DECODE_EMULATE (0xc000, 0x4000, PROBES_T16_LDRHSTRH),
+ DECODE_EMULATE (0xe800, 0x6800, PROBES_T16_LDRH),
+
/* STRH (immediate, Thumb) 1000 0xxx xxxx xxxx */
+ DECODE_EMULATE (0xf800, 0x8000, PROBES_T16_STRH),
/* LDRH (immediate, Thumb) 1000 1xxx xxxx xxxx */
- DECODE_EMULATE (0xf000, 0x8000, PROBES_T16_LDRHSTRH),
+ DECODE_EMULATE (0xf800, 0x8800, PROBES_T16_LDRH),
/* STR (immediate, Thumb) 1001 0xxx xxxx xxxx */
+ DECODE_SIMULATE (0xf800, 0x9000, PROBES_T16_STR),
/* LDR (immediate, Thumb) 1001 1xxx xxxx xxxx */
- DECODE_SIMULATE (0xf000, 0x9000, PROBES_T16_LDRSTR),
+ DECODE_SIMULATE (0xf800, 0x9800, PROBES_T16_LDR),
/*
* Generate PC-/SP-relative address
@@ -810,8 +847,9 @@ const union decode_item probes_decode_thumb16_table[] = {
DECODE_TABLE (0xf000, 0xb000, t16_table_1011),
/* STM 1100 0xxx xxxx xxxx */
+ DECODE_EMULATE (0xf800, 0xc000, PROBES_T16_STM),
/* LDM 1100 1xxx xxxx xxxx */
- DECODE_EMULATE (0xf000, 0xc000, PROBES_T16_LDMSTM),
+ DECODE_EMULATE (0xf800, 0xc800, PROBES_T16_LDM),
/*
* Conditional branch, and Supervisor Call
diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
index 2658d95..a9c65c9 100644
--- a/arch/arm/kernel/probes-thumb.h
+++ b/arch/arm/kernel/probes-thumb.h
@@ -30,8 +30,10 @@
enum probes_t32_action {
PROBES_T32_EMULATE_NONE,
PROBES_T32_SIMULATE_NOP,
- PROBES_T32_LDMSTM,
- PROBES_T32_LDRDSTRD,
+ PROBES_T32_LDM,
+ PROBES_T32_STM,
+ PROBES_T32_LDRD,
+ PROBES_T32_STRD,
PROBES_T32_TABLE_BRANCH,
PROBES_T32_TST,
PROBES_T32_CMP,
@@ -50,7 +52,8 @@ enum probes_t32_action {
PROBES_T32_BRANCH,
PROBES_T32_PLDI,
PROBES_T32_LDR_LIT,
- PROBES_T32_LDRSTR,
+ PROBES_T32_LDR,
+ PROBES_T32_STR,
PROBES_T32_SIGN_EXTEND,
PROBES_T32_MEDIA,
PROBES_T32_REVERSE,
@@ -75,10 +78,13 @@ enum probes_t16_action {
PROBES_T16_BLX,
PROBES_T16_HIREGOPS,
PROBES_T16_LDR_LIT,
- PROBES_T16_LDRHSTRH,
- PROBES_T16_LDRSTR,
+ PROBES_T16_LDRH,
+ PROBES_T16_STRH,
+ PROBES_T16_LDR,
+ PROBES_T16_STR,
PROBES_T16_ADR,
- PROBES_T16_LDMSTM,
+ PROBES_T16_LDM,
+ PROBES_T16_STM,
PROBES_T16_BRANCH_COND,
PROBES_T16_BRANCH,
NUM_PROBES_T16_ACTIONS
diff --git a/arch/arm/kernel/uprobes-arm.c b/arch/arm/kernel/uprobes-arm.c
index 0c0f299..0a8caa3 100644
--- a/arch/arm/kernel/uprobes-arm.c
+++ b/arch/arm/kernel/uprobes-arm.c
@@ -207,7 +207,8 @@ const struct decode_action uprobes_probes_actions[] = {
[PROBES_MUL1] = {.handler = probes_simulate_nop},
[PROBES_MUL2] = {.handler = probes_simulate_nop},
[PROBES_SWP] = {.handler = probes_simulate_nop},
- [PROBES_LDRSTRD] = {.decoder = decode_pc_ro},
+ [PROBES_LDRD] = {.decoder = decode_pc_ro},
+ [PROBES_STRD] = {.decoder = decode_pc_ro},
[PROBES_LOAD_EXTRA] = {.decoder = decode_pc_ro},
[PROBES_LOAD] = {.decoder = decode_ldr},
[PROBES_STORE_EXTRA] = {.decoder = decode_pc_ro},
@@ -230,5 +231,6 @@ const struct decode_action uprobes_probes_actions[] = {
[PROBES_MUL_ADD] = {.handler = probes_simulate_nop},
[PROBES_BITFIELD] = {.handler = probes_simulate_nop},
[PROBES_BRANCH] = {.handler = simulate_bbl},
- [PROBES_LDMSTM] = {.decoder = uprobe_decode_ldmstm}
+ [PROBES_LDM] = {.decoder = uprobe_decode_ldmstm},
+ [PROBES_STM] = {.decoder = uprobe_decode_ldmstm}
};
--
1.8.4
This patch is generated simply using:
$ sed -i "s/union decode_action/struct decode_action/g" `grep decode_action * -rl`
Which allows futher expansion to decode_action.
Signed-off-by: Wang Nan <[email protected]>
---
arch/arm/kernel/kprobes-arm.c | 2 +-
arch/arm/kernel/kprobes-thumb.c | 4 ++--
arch/arm/kernel/kprobes.c | 2 +-
arch/arm/kernel/kprobes.h | 8 ++++----
arch/arm/kernel/probes-arm.c | 2 +-
arch/arm/kernel/probes-arm.h | 2 +-
arch/arm/kernel/probes-thumb.c | 4 ++--
arch/arm/kernel/probes-thumb.h | 4 ++--
arch/arm/kernel/probes.c | 2 +-
arch/arm/kernel/probes.h | 4 ++--
arch/arm/kernel/uprobes-arm.c | 2 +-
arch/arm/kernel/uprobes.h | 2 +-
12 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/arch/arm/kernel/kprobes-arm.c b/arch/arm/kernel/kprobes-arm.c
index ac300c6..6df9f1f 100644
--- a/arch/arm/kernel/kprobes-arm.c
+++ b/arch/arm/kernel/kprobes-arm.c
@@ -301,7 +301,7 @@ emulate_rdlo12rdhi16rn0rm8_rwflags_nopc(probes_opcode_t insn,
regs->ARM_cpsr = (regs->ARM_cpsr & ~APSR_MASK) | (cpsr & APSR_MASK);
}
-const union decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
+const struct decode_action kprobes_arm_actions[NUM_PROBES_ARM_ACTIONS] = {
[PROBES_EMULATE_NONE] = {.handler = probes_emulate_none},
[PROBES_SIMULATE_NOP] = {.handler = probes_simulate_nop},
[PROBES_PRELOAD_IMM] = {.handler = probes_simulate_nop},
diff --git a/arch/arm/kernel/kprobes-thumb.c b/arch/arm/kernel/kprobes-thumb.c
index 9495d7f..0672457 100644
--- a/arch/arm/kernel/kprobes-thumb.c
+++ b/arch/arm/kernel/kprobes-thumb.c
@@ -611,7 +611,7 @@ t16_decode_pop(probes_opcode_t insn, struct arch_probes_insn *asi,
return INSN_GOOD;
}
-const union decode_action kprobes_t16_actions[NUM_PROBES_T16_ACTIONS] = {
+const struct decode_action kprobes_t16_actions[NUM_PROBES_T16_ACTIONS] = {
[PROBES_T16_ADD_SP] = {.handler = t16_simulate_add_sp_imm},
[PROBES_T16_CBZ] = {.handler = t16_simulate_cbz},
[PROBES_T16_SIGN_EXTEND] = {.handler = t16_emulate_loregs_rwflags},
@@ -634,7 +634,7 @@ const union decode_action kprobes_t16_actions[NUM_PROBES_T16_ACTIONS] = {
[PROBES_T16_BRANCH] = {.handler = t16_simulate_branch},
};
-const union decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
+const struct decode_action kprobes_t32_actions[NUM_PROBES_T32_ACTIONS] = {
[PROBES_T32_LDMSTM] = {.decoder = t32_decode_ldmstm},
[PROBES_T32_LDRDSTRD] = {.handler = t32_emulate_ldrdstrd},
[PROBES_T32_TABLE_BRANCH] = {.handler = t32_simulate_table_branch},
diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
index 6d64420..028159c 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -59,7 +59,7 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
unsigned long addr = (unsigned long)p->addr;
bool thumb;
kprobe_decode_insn_t *decode_insn;
- const union decode_action *actions;
+ const struct decode_action *actions;
int is;
if (in_exception_text(addr))
diff --git a/arch/arm/kernel/kprobes.h b/arch/arm/kernel/kprobes.h
index 9a2712e..34db1eb 100644
--- a/arch/arm/kernel/kprobes.h
+++ b/arch/arm/kernel/kprobes.h
@@ -36,16 +36,16 @@ kprobe_decode_ldmstm(kprobe_opcode_t insn, struct arch_probes_insn *asi,
typedef enum probes_insn (kprobe_decode_insn_t)(probes_opcode_t,
struct arch_probes_insn *,
bool,
- const union decode_action *);
+ const struct decode_action *);
#ifdef CONFIG_THUMB2_KERNEL
-extern const union decode_action kprobes_t32_actions[];
-extern const union decode_action kprobes_t16_actions[];
+extern const struct decode_action kprobes_t32_actions[];
+extern const struct decode_action kprobes_t16_actions[];
#else /* !CONFIG_THUMB2_KERNEL */
-extern const union decode_action kprobes_arm_actions[];
+extern const struct decode_action kprobes_arm_actions[];
#endif
diff --git a/arch/arm/kernel/probes-arm.c b/arch/arm/kernel/probes-arm.c
index 8eaef81..3d00aa7 100644
--- a/arch/arm/kernel/probes-arm.c
+++ b/arch/arm/kernel/probes-arm.c
@@ -725,7 +725,7 @@ static void __kprobes arm_singlestep(probes_opcode_t insn,
*/
enum probes_insn __kprobes
arm_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
- bool emulate, const union decode_action *actions)
+ bool emulate, const struct decode_action *actions)
{
asi->insn_singlestep = arm_singlestep;
asi->insn_check_cc = probes_condition_checks[insn>>28];
diff --git a/arch/arm/kernel/probes-arm.h b/arch/arm/kernel/probes-arm.h
index ace6572..6ecc25a 100644
--- a/arch/arm/kernel/probes-arm.h
+++ b/arch/arm/kernel/probes-arm.h
@@ -68,6 +68,6 @@ extern const union decode_item probes_decode_arm_table[];
enum probes_insn arm_probes_decode_insn(probes_opcode_t,
struct arch_probes_insn *, bool emulate,
- const union decode_action *actions);
+ const struct decode_action *actions);
#endif
diff --git a/arch/arm/kernel/probes-thumb.c b/arch/arm/kernel/probes-thumb.c
index 4131351..72aa217 100644
--- a/arch/arm/kernel/probes-thumb.c
+++ b/arch/arm/kernel/probes-thumb.c
@@ -863,7 +863,7 @@ static void __kprobes thumb32_singlestep(probes_opcode_t opcode,
enum probes_insn __kprobes
thumb16_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
- bool emulate, const union decode_action *actions)
+ bool emulate, const struct decode_action *actions)
{
asi->insn_singlestep = thumb16_singlestep;
asi->insn_check_cc = thumb_check_cc;
@@ -873,7 +873,7 @@ thumb16_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
enum probes_insn __kprobes
thumb32_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
- bool emulate, const union decode_action *actions)
+ bool emulate, const struct decode_action *actions)
{
asi->insn_singlestep = thumb32_singlestep;
asi->insn_check_cc = thumb_check_cc;
diff --git a/arch/arm/kernel/probes-thumb.h b/arch/arm/kernel/probes-thumb.h
index 7c6f6eb..2658d95 100644
--- a/arch/arm/kernel/probes-thumb.h
+++ b/arch/arm/kernel/probes-thumb.h
@@ -89,9 +89,9 @@ extern const union decode_item probes_decode_thumb16_table[];
enum probes_insn __kprobes
thumb16_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
- bool emulate, const union decode_action *actions);
+ bool emulate, const struct decode_action *actions);
enum probes_insn __kprobes
thumb32_probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
- bool emulate, const union decode_action *actions);
+ bool emulate, const struct decode_action *actions);
#endif
diff --git a/arch/arm/kernel/probes.c b/arch/arm/kernel/probes.c
index a8ab540..ec030b8 100644
--- a/arch/arm/kernel/probes.c
+++ b/arch/arm/kernel/probes.c
@@ -388,7 +388,7 @@ static const int decode_struct_sizes[NUM_DECODE_TYPES] = {
int __kprobes
probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
const union decode_item *table, bool thumb,
- bool emulate, const union decode_action *actions)
+ bool emulate, const struct decode_action *actions)
{
const struct decode_header *h = (struct decode_header *)table;
const struct decode_header *next;
diff --git a/arch/arm/kernel/probes.h b/arch/arm/kernel/probes.h
index dba9f24..739c2a2 100644
--- a/arch/arm/kernel/probes.h
+++ b/arch/arm/kernel/probes.h
@@ -309,7 +309,7 @@ typedef enum probes_insn (probes_custom_decode_t)(probes_opcode_t,
struct arch_probes_insn *,
const struct decode_header *);
-union decode_action {
+struct decode_action {
probes_insn_handler_t *handler;
probes_custom_decode_t *decoder;
};
@@ -402,6 +402,6 @@ probes_insn_handler_t probes_emulate_none;
int __kprobes
probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
const union decode_item *table, bool thumb, bool emulate,
- const union decode_action *actions);
+ const struct decode_action *actions);
#endif
diff --git a/arch/arm/kernel/uprobes-arm.c b/arch/arm/kernel/uprobes-arm.c
index d3b655f..0c0f299 100644
--- a/arch/arm/kernel/uprobes-arm.c
+++ b/arch/arm/kernel/uprobes-arm.c
@@ -194,7 +194,7 @@ uprobe_decode_ldmstm(probes_opcode_t insn,
return INSN_GOOD;
}
-const union decode_action uprobes_probes_actions[] = {
+const struct decode_action uprobes_probes_actions[] = {
[PROBES_EMULATE_NONE] = {.handler = probes_simulate_nop},
[PROBES_SIMULATE_NOP] = {.handler = probes_simulate_nop},
[PROBES_PRELOAD_IMM] = {.handler = probes_simulate_nop},
diff --git a/arch/arm/kernel/uprobes.h b/arch/arm/kernel/uprobes.h
index 1d0c12d..57b4373 100644
--- a/arch/arm/kernel/uprobes.h
+++ b/arch/arm/kernel/uprobes.h
@@ -30,6 +30,6 @@ enum probes_insn
decode_pc_ro(probes_opcode_t insn, struct arch_probes_insn *asi,
const struct decode_header *d);
-extern const union decode_action uprobes_probes_actions[];
+extern const struct decode_action uprobes_probes_actions[];
#endif
--
1.8.4
This patch prohibit probing instructions for which the stack
requirement are unable to be determined statically. Some test cases
are found not work again after the modification, this patch also
removes them.
Signed-off-by: Wang Nan <[email protected]>
---
arch/arm/kernel/kprobes-test-arm.c | 16 ++++++++++------
arch/arm/kernel/kprobes.c | 8 ++++++++
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
index 264c064..59f9b25 100644
--- a/arch/arm/kernel/kprobes-test-arm.c
+++ b/arch/arm/kernel/kprobes-test-arm.c
@@ -476,7 +476,8 @@ void kprobe_arm_test_cases(void)
TEST_GROUP("Extra load/store instructions")
TEST_RPR( "strh r",0, VAL1,", [r",1, 48,", -r",2, 24,"]")
- TEST_RPR( "streqh r",14,VAL2,", [r",13,0, ", r",12, 48,"]")
+ TEST_RPR( "streqh r",14,VAL2,", [r",11,0, ", r",12, 48,"]")
+ TEST_UNSUPPORTED( "streqh r14, [r13, r12]")
TEST_RPR( "strh r",1, VAL1,", [r",2, 24,", r",3, 48,"]!")
TEST_RPR( "strneh r",12,VAL2,", [r",11,48,", -r",10,24,"]!")
TEST_RPR( "strh r",2, VAL1,", [r",3, 24,"], r",4, 48,"")
@@ -565,7 +566,8 @@ void kprobe_arm_test_cases(void)
#if __LINUX_ARM_ARCH__ >= 5
TEST_RPR( "strd r",0, VAL1,", [r",1, 48,", -r",2,24,"]")
- TEST_RPR( "strccd r",8, VAL2,", [r",13,0, ", r",12,48,"]")
+ TEST_RPR( "strccd r",8, VAL2,", [r",11,0, ", r",12,48,"]")
+ TEST_UNSUPPORTED( "strccd r8, [r13, r12]")
TEST_RPR( "strd r",4, VAL1,", [r",2, 24,", r",3, 48,"]!")
TEST_RPR( "strcsd r",12,VAL2,", [r",11,48,", -r",10,24,"]!")
TEST_RPR( "strd r",2, VAL1,", [r",5, 24,"], r",4,48,"")
@@ -639,13 +641,15 @@ void kprobe_arm_test_cases(void)
TEST_RP( "str"byte" r",2, VAL1,", [r",3, 24,"], #48") \
TEST_RP( "str"byte" r",10,VAL2,", [r",9, 64,"], #-48") \
TEST_RPR("str"byte" r",0, VAL1,", [r",1, 48,", -r",2, 24,"]") \
- TEST_RPR("str"byte" r",14,VAL2,", [r",13,0, ", r",12, 48,"]") \
+ TEST_RPR("str"byte" r",14,VAL2,", [r",11,0, ", r",12, 48,"]") \
+ TEST_UNSUPPORTED("str"byte" r14, [r13, r12]") \
TEST_RPR("str"byte" r",1, VAL1,", [r",2, 24,", r",3, 48,"]!") \
TEST_RPR("str"byte" r",12,VAL2,", [r",11,48,", -r",10,24,"]!") \
TEST_RPR("str"byte" r",2, VAL1,", [r",3, 24,"], r",4, 48,"") \
TEST_RPR("str"byte" r",10,VAL2,", [r",9, 48,"], -r",11,24,"") \
TEST_RPR("str"byte" r",0, VAL1,", [r",1, 24,", r",2, 32,", asl #1]")\
- TEST_RPR("str"byte" r",14,VAL2,", [r",13,0, ", r",12, 32,", lsr #2]")\
+ TEST_RPR("str"byte" r",14,VAL2,", [r",11,0, ", r",12, 32,", lsr #2]")\
+ TEST_UNSUPPORTED("str"byte" r14, [r13, r12, lsr #2]")\
TEST_RPR("str"byte" r",1, VAL1,", [r",2, 24,", r",3, 32,", asr #3]!")\
TEST_RPR("str"byte" r",12,VAL2,", [r",11,24,", r",10, 4,", ror #31]!")\
TEST_P( "ldr"byte" r0, [r",0, 24,", #-2]") \
@@ -669,12 +673,12 @@ void kprobe_arm_test_cases(void)
LOAD_STORE("")
TEST_P( "str pc, [r",0,0,", #15*4]")
- TEST_R( "str pc, [sp, r",2,15*4,"]")
+ TEST_UNSUPPORTED( "str pc, [sp, r2]")
TEST_BF( "ldr pc, [sp, #15*4]")
TEST_BF_R("ldr pc, [sp, r",2,15*4,"]")
TEST_P( "str sp, [r",0,0,", #13*4]")
- TEST_R( "str sp, [sp, r",2,13*4,"]")
+ TEST_UNSUPPORTED( "str sp, [sp, r2]")
TEST_BF( "ldr sp, [sp, #13*4]")
TEST_BF_R("ldr sp, [sp, r",2,13*4,"]")
diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
index 028159c..afbb3e5 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -111,6 +111,14 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
break;
}
+ /*
+ * Unable to instrument insn like 'str r0, [sp, +/-r1]'.
+ * __und_svc protects 64 bytes stack, so instrumenting insn
+ * likes 'str r0, [sp, #-68]' should be prohibited.
+ */
+ if ((p->ainsn.stack_space < 0) || (p->ainsn.stack_space > 64))
+ return -EINVAL;
+
return 0;
}
--
1.8.4
This patch introdces a 'checker' field to decode_action, and calls
checkers when instruction decoding. This allows further analysis
for specific instructions.
Signed-off-by: Wang Nan <[email protected]>
---
arch/arm/kernel/probes.c | 10 ++++++++++
arch/arm/kernel/probes.h | 10 ++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/probes.c b/arch/arm/kernel/probes.c
index ec030b8..6164b4d 100644
--- a/arch/arm/kernel/probes.c
+++ b/arch/arm/kernel/probes.c
@@ -393,6 +393,7 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
const struct decode_header *h = (struct decode_header *)table;
const struct decode_header *next;
bool matched = false;
+ probes_opcode_t origin_insn = insn;
if (emulate)
insn = prepare_emulated_insn(insn, asi, thumb);
@@ -423,17 +424,26 @@ probes_decode_insn(probes_opcode_t insn, struct arch_probes_insn *asi,
case DECODE_TYPE_CUSTOM: {
struct decode_custom *d = (struct decode_custom *)h;
+ probes_check_t *checker = actions[d->decoder.action].checker;
+ if (checker && (checker(origin_insn, asi, h) == INSN_REJECTED))
+ return INSN_REJECTED;
return actions[d->decoder.action].decoder(insn, asi, h);
}
case DECODE_TYPE_SIMULATE: {
struct decode_simulate *d = (struct decode_simulate *)h;
+ probes_check_t *checker = actions[d->handler.action].checker;
+ if (checker && (checker(origin_insn, asi, h) == INSN_REJECTED))
+ return INSN_REJECTED;
asi->insn_handler = actions[d->handler.action].handler;
return INSN_GOOD_NO_SLOT;
}
case DECODE_TYPE_EMULATE: {
struct decode_emulate *d = (struct decode_emulate *)h;
+ probes_check_t *checker = actions[d->handler.action].checker;
+ if (checker && (checker(origin_insn, asi, h) == INSN_REJECTED))
+ return INSN_REJECTED;
if (!emulate)
return actions[d->handler.action].decoder(insn,
diff --git a/arch/arm/kernel/probes.h b/arch/arm/kernel/probes.h
index 739c2a2..c56dd3d 100644
--- a/arch/arm/kernel/probes.h
+++ b/arch/arm/kernel/probes.h
@@ -309,9 +309,15 @@ typedef enum probes_insn (probes_custom_decode_t)(probes_opcode_t,
struct arch_probes_insn *,
const struct decode_header *);
+typedef enum probes_insn (probes_check_t)(probes_opcode_t,
+ struct arch_probes_insn *,
+ const struct decode_header *);
struct decode_action {
- probes_insn_handler_t *handler;
- probes_custom_decode_t *decoder;
+ probes_check_t *checker;
+ union {
+ probes_insn_handler_t *handler;
+ probes_custom_decode_t *decoder;
+ };
};
#define DECODE_END \
--
1.8.4
This patch introduce kprobeopt for ARM 32.
Limitations:
- Currently only kernel compiled with ARM ISA is supported.
- Offset between probe point and optinsn slot must not larger than
32MiB. Masami Hiramatsu suggests replacing 2 words, it will make
things complex. Futher patch can make such optimization.
Kprobe opt on ARM is relatively simpler than kprobe opt on x86 because
ARM instruction is always 4 bytes aligned and 4 bytes long. This patch
replace probed instruction by a 'b', branch to trampoline code and then
calls optimized_callback(). optimized_callback() calls opt_pre_handler()
to execute kprobe handler. It also emulate/simulate replaced instruction.
When unregistering kprobe, the deferred manner of unoptimizer may leave
branch instruction before optimizer is called. Different from x86_64,
which only copy the probed insn after optprobe_template_end and
reexecute them, this patch call singlestep to emulate/simulate the insn
directly. Futher patch can optimize this behavior.
v1 -> v2:
- Improvement: if replaced instruction is conditional, generate a
conditional branch instruction for it;
- Introduces RELATIVEJUMP_OPCODES due to ARM kprobe_opcode_t is 4
bytes;
- Removes size field in struct arch_optimized_insn;
- Use arm_gen_branch() to generate branch instruction;
- Remove all recover logic: ARM doesn't use tail buffer, no need
recover replaced instructions like x86;
- Remove incorrect CONFIG_THUMB checking;
- can_optimize() always returns true if address is well aligned;
- Improve optimized_callback: using opt_pre_handler();
- Bugfix: correct range checking code and improve comments;
- Fix commit message.
v2 -> v3:
- Rename RELATIVEJUMP_OPCODES to MAX_COPIED_INSNS;
- Remove unneeded checking:
arch_check_optimized_kprobe(), can_optimize();
- Add missing flush_icache_range() in arch_prepare_optimized_kprobe();
- Remove unneeded 'return;'.
v3 -> v4:
- Use __mem_to_opcode_arm() to translate copied_insn to ensure it
works in big endian kernel;
- Replace 'nop' placeholder in trampoline code template with
'.long 0' to avoid confusion: reader may regard 'nop' as an
instruction, but it is value in fact.
v4 -> v5:
- Don't optimize stack store operations.
- Introduce prepared field to arch_optimized_insn to indicate whether
it is prepared. Similar to size field with x86. See v1 -> v2.
v5 -> v6:
- Dynamically reserve stack according to instruction.
- Rename: kprobes-opt.c -> kprobes-opt-arm.c.
- Set op->optinsn.insn after all works are done.
Signed-off-by: Wang Nan <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Jon Medhurst (Tixy) <[email protected]>
Cc: Russell King - ARM Linux <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/kprobes.h | 26 ++++
arch/arm/kernel/Makefile | 3 +-
arch/arm/kernel/kprobes-opt-arm.c | 285 ++++++++++++++++++++++++++++++++++++++
4 files changed, 314 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/kernel/kprobes-opt-arm.c
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89c4b5c..8281cea 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -59,6 +59,7 @@ config ARM
select HAVE_MEMBLOCK
select HAVE_MOD_ARCH_SPECIFIC if ARM_UNWIND
select HAVE_OPROFILE if (HAVE_PERF_EVENTS)
+ select HAVE_OPTPROBES if (!THUMB2_KERNEL)
select HAVE_PERF_EVENTS
select HAVE_PERF_REGS
select HAVE_PERF_USER_STACK_DUMP
diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h
index 49fa0df..a05297f 100644
--- a/arch/arm/include/asm/kprobes.h
+++ b/arch/arm/include/asm/kprobes.h
@@ -51,5 +51,31 @@ int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr);
int kprobe_exceptions_notify(struct notifier_block *self,
unsigned long val, void *data);
+/* optinsn template addresses */
+extern __visible kprobe_opcode_t optprobe_template_entry;
+extern __visible kprobe_opcode_t optprobe_template_val;
+extern __visible kprobe_opcode_t optprobe_template_call;
+extern __visible kprobe_opcode_t optprobe_template_end;
+
+#define MAX_OPTIMIZED_LENGTH (4)
+#define MAX_OPTINSN_SIZE \
+ (((unsigned long)&optprobe_template_end - \
+ (unsigned long)&optprobe_template_entry))
+#define RELATIVEJUMP_SIZE (4)
+
+struct arch_optimized_insn {
+ /*
+ * copy of the original instructions.
+ * Different from x86, ARM kprobe_opcode_t is u32.
+ */
+#define MAX_COPIED_INSN ((RELATIVEJUMP_SIZE) / sizeof(kprobe_opcode_t))
+ kprobe_opcode_t copied_insn[MAX_COPIED_INSN];
+ /* detour code buffer */
+ kprobe_opcode_t *insn;
+ /*
+ * we always copies one instruction on arm32,
+ * size always be 4, so no size field.
+ */
+};
#endif /* _ARM_KPROBES_H */
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 38ddd9f..2fcd980 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -52,11 +52,12 @@ obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o insn.o
obj-$(CONFIG_JUMP_LABEL) += jump_label.o insn.o patch.o
obj-$(CONFIG_KEXEC) += machine_kexec.o relocate_kernel.o
obj-$(CONFIG_UPROBES) += probes.o probes-arm.o uprobes.o uprobes-arm.o
-obj-$(CONFIG_KPROBES) += probes.o kprobes.o kprobes-common.o patch.o
+obj-$(CONFIG_KPROBES) += probes.o kprobes.o kprobes-common.o patch.o insn.o
ifdef CONFIG_THUMB2_KERNEL
obj-$(CONFIG_KPROBES) += kprobes-thumb.o probes-thumb.o
else
obj-$(CONFIG_KPROBES) += kprobes-arm.o probes-arm.o
+obj-$(CONFIG_OPTPROBES) += kprobes-opt-arm.o
endif
obj-$(CONFIG_ARM_KPROBES_TEST) += test-kprobes.o
test-kprobes-objs := kprobes-test.o
diff --git a/arch/arm/kernel/kprobes-opt-arm.c b/arch/arm/kernel/kprobes-opt-arm.c
new file mode 100644
index 0000000..55eb3ea
--- /dev/null
+++ b/arch/arm/kernel/kprobes-opt-arm.c
@@ -0,0 +1,285 @@
+/*
+ * Kernel Probes Jump Optimization (Optprobes)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) IBM Corporation, 2002, 2004
+ * Copyright (C) Hitachi Ltd., 2012
+ * Copyright (C) Huawei Inc., 2014
+ */
+
+#include <linux/kprobes.h>
+#include <linux/jump_label.h>
+#include <asm/kprobes.h>
+#include <asm/cacheflush.h>
+/* for arm_gen_branch */
+#include "insn.h"
+/* for patch_text */
+#include "patch.h"
+
+/*
+ * NOTE: the first sub and add instruction will be modified according
+ * to the stack cost of the instruction.
+ */
+asm (
+ ".global optprobe_template_entry\n"
+ "optprobe_template_entry:\n"
+ " sub sp, sp, #0xff\n"
+ " stmia sp, {r0 - r14} \n"
+ " add r3, sp, #0xff\n"
+ " str r3, [sp, #52]\n"
+ " mrs r4, cpsr\n"
+ " str r4, [sp, #64]\n"
+ " mov r1, sp\n"
+ " ldr r0, 1f\n"
+ " ldr r2, 2f\n"
+ " blx r2\n"
+ " ldr r1, [sp, #64]\n"
+ " msr cpsr_fs, r1\n"
+ " ldmia sp, {r0 - r15}\n"
+ ".global optprobe_template_val\n"
+ "optprobe_template_val:\n"
+ "1: .long 0\n"
+ ".global optprobe_template_call\n"
+ "optprobe_template_call:\n"
+ "2: .long 0\n"
+ ".global optprobe_template_end\n"
+ "optprobe_template_end:\n");
+
+#define TMPL_VAL_IDX \
+ ((long)&optprobe_template_val - (long)&optprobe_template_entry)
+#define TMPL_CALL_IDX \
+ ((long)&optprobe_template_call - (long)&optprobe_template_entry)
+#define TMPL_END_IDX \
+ ((long)&optprobe_template_end - (long)&optprobe_template_entry)
+
+/*
+ * ARM can always optimize an instruction when using ARM ISA, except
+ * instructions like 'str r0, [sp, r1]' which store to stack and unable
+ * to determine stack space consumption statically.
+ */
+int arch_prepared_optinsn(struct arch_optimized_insn *optinsn)
+{
+ return optinsn->insn != NULL;
+}
+
+/*
+ * In ARM ISA, kprobe opt always replace one instruction (4 bytes
+ * aligned and 4 bytes long). It is impossiable to encounter another
+ * kprobe in the address range. So always return 0.
+ */
+int arch_check_optimized_kprobe(struct optimized_kprobe *op)
+{
+ return 0;
+}
+
+/* Caller must ensure addr & 3 == 0 */
+static int can_optimize(struct optimized_kprobe *op)
+{
+ if (op->kp.ainsn.stack_space < 0)
+ return 0;
+ /*
+ * 255 is the biggest imm can be used in 'sub r0, r0, #<imm>'.
+ * Number larger than 255 needs special encoding.
+ */
+ if (op->kp.ainsn.stack_space > 255 - sizeof(struct pt_regs))
+ return 0;
+ return 1;
+}
+
+/* Free optimized instruction slot */
+static void
+__arch_remove_optimized_kprobe(struct optimized_kprobe *op, int dirty)
+{
+ if (op->optinsn.insn) {
+ free_optinsn_slot(op->optinsn.insn, dirty);
+ op->optinsn.insn = NULL;
+ }
+}
+
+extern void kprobe_handler(struct pt_regs *regs);
+
+static void
+optimized_callback(struct optimized_kprobe *op, struct pt_regs *regs)
+{
+ unsigned long flags;
+ struct kprobe *p = &op->kp;
+ struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+ /* Save skipped registers */
+ regs->ARM_pc = (unsigned long)op->kp.addr;
+ regs->ARM_ORIG_r0 = ~0UL;
+
+ local_irq_save(flags);
+
+ if (kprobe_running()) {
+ kprobes_inc_nmissed_count(&op->kp);
+ } else {
+ __this_cpu_write(current_kprobe, &op->kp);
+ kcb->kprobe_status = KPROBE_HIT_ACTIVE;
+ opt_pre_handler(&op->kp, regs);
+ __this_cpu_write(current_kprobe, NULL);
+ }
+
+ /* In each case, we must singlestep the replaced instruction. */
+ op->kp.ainsn.insn_singlestep(p->opcode, &p->ainsn, regs);
+
+ local_irq_restore(flags);
+}
+
+int arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
+{
+ u8 *buf;
+ unsigned long *code;
+ unsigned long rel_chk;
+ unsigned long val;
+ unsigned long stack_protect = sizeof(struct pt_regs);
+
+ if (!can_optimize(op))
+ return -EILSEQ;
+
+ buf = (u8 *)get_optinsn_slot();
+ if (!buf)
+ return -ENOMEM;
+
+ /*
+ * Verify if the address gap is in 32MiB range, because this uses
+ * a relative jump.
+ *
+ * kprobe opt use a 'b' instruction to branch to optinsn.insn.
+ * According to ARM manual, branch instruction is:
+ *
+ * 31 28 27 24 23 0
+ * +------+---+---+---+---+----------------+
+ * | cond | 1 | 0 | 1 | 0 | imm24 |
+ * +------+---+---+---+---+----------------+
+ *
+ * imm24 is a signed 24 bits integer. The real branch offset is computed
+ * by: imm32 = SignExtend(imm24:'00', 32);
+ *
+ * So the maximum forward branch should be:
+ * (0x007fffff << 2) = 0x01fffffc = 0x1fffffc
+ * The maximum backword branch should be:
+ * (0xff800000 << 2) = 0xfe000000 = -0x2000000
+ *
+ * We can simply check (rel & 0xfe000003):
+ * if rel is positive, (rel & 0xfe000000) shoule be 0
+ * if rel is negitive, (rel & 0xfe000000) should be 0xfe000000
+ * the last '3' is used for alignment checking.
+ */
+ rel_chk = (unsigned long)((long)buf -
+ (long)op->kp.addr + 8) & 0xfe000003;
+
+ if ((rel_chk != 0) && (rel_chk != 0xfe000000)) {
+ __arch_remove_optimized_kprobe(op, 0);
+ return -ERANGE;
+ }
+
+ /* Copy arch-dep-instance from template. */
+ memcpy(buf, &optprobe_template_entry, TMPL_END_IDX);
+
+ /* Adjust buffer according to instruction. */
+ BUG_ON(op->kp.ainsn.stack_space < 0);
+ stack_protect += op->kp.ainsn.stack_space;
+
+ /* Should have been filtered by can_optimize(). */
+ BUG_ON(stack_protect > 255);
+
+ /* Create a 'sub sp, sp, #<stack_protect>' */
+ code = (unsigned long *)(buf);
+ code[0] = __opcode_to_mem_arm(0xe24dd000 | stack_protect);
+ /* Create a 'add r3, sp, #<stack_protect>' */
+ code[2] = __opcode_to_mem_arm(0xe28d3000 | stack_protect);
+
+ /* Set probe information */
+ val = (unsigned long)op;
+ memcpy(buf + TMPL_VAL_IDX, &val, sizeof(val));
+
+ /* Set probe function call */
+ val = (unsigned long)optimized_callback;
+ memcpy(buf + TMPL_CALL_IDX, &val, sizeof(val));
+
+ flush_icache_range((unsigned long)buf,
+ (unsigned long)buf + TMPL_END_IDX);
+
+ /* Set op->optinsn.insn means prepared */
+ op->optinsn.insn = (kprobe_opcode_t *)buf;
+ return 0;
+}
+
+void arch_optimize_kprobes(struct list_head *oplist)
+{
+ struct optimized_kprobe *op, *tmp;
+
+ list_for_each_entry_safe(op, tmp, oplist, list) {
+ unsigned long insn;
+ WARN_ON(kprobe_disabled(&op->kp));
+
+ /*
+ * Backup instructions which will be replaced
+ * by jump address
+ */
+ memcpy(op->optinsn.copied_insn, op->kp.addr,
+ RELATIVEJUMP_SIZE);
+
+ insn = arm_gen_branch((unsigned long)op->kp.addr,
+ (unsigned long)op->optinsn.insn);
+ BUG_ON(insn == 0);
+
+ /*
+ * Make it a conditional branch if replaced insn
+ * is consitional
+ */
+ insn = (__mem_to_opcode_arm(
+ op->optinsn.copied_insn[0]) & 0xf0000000) |
+ (insn & 0x0fffffff);
+
+ patch_text(op->kp.addr, insn);
+
+ list_del_init(&op->list);
+ }
+}
+
+void arch_unoptimize_kprobe(struct optimized_kprobe *op)
+{
+ arch_arm_kprobe(&op->kp);
+}
+
+/*
+ * Recover original instructions and breakpoints from relative jumps.
+ * Caller must call with locking kprobe_mutex.
+ */
+void arch_unoptimize_kprobes(struct list_head *oplist,
+ struct list_head *done_list)
+{
+ struct optimized_kprobe *op, *tmp;
+
+ list_for_each_entry_safe(op, tmp, oplist, list) {
+ arch_unoptimize_kprobe(op);
+ list_move(&op->list, done_list);
+ }
+}
+
+int arch_within_optimized_kprobe(struct optimized_kprobe *op,
+ unsigned long addr)
+{
+ return ((unsigned long)op->kp.addr <= addr &&
+ (unsigned long)op->kp.addr + RELATIVEJUMP_SIZE > addr);
+}
+
+void arch_remove_optimized_kprobe(struct optimized_kprobe *op)
+{
+ __arch_remove_optimized_kprobe(op, 1);
+}
--
1.8.4
(2014/10/22 20:31), Wang Nan wrote:
> Previous 5 version of ARM OPTPROBES patches are unable to deal with
> stack storing instructions correctly. V5 patches disallow optimizing
> every protential stack store instructions based on pessimistic
> assumption. Which, as Tixy comments, 'excludes the main use of
> kprobes'. (https://lkml.org/lkml/2014/8/29/117 )
>
> The main obstacle which prevents us from computing stack requirement is
> the missing of per-instruction decoder in probes_decode_insn() and its
> friends. Only part of instructions have their decoders (and not in
> each case).
>
> In this patch series, I propose 'checker', which allows us define
> functions for each type of instruction, extract more information. Stack
> consumption computing is an example. Checker can be further employed to
> determine whether one instruction is possible to execute directy in
> optimized kprobe. I'd like to expand current checker framework by
> chaining checkers together. After that, I believe most of ARM
> instructions can be executed directly like x86, kprobe performace can be
> improved.
>
> The first 3 patches introduces checker. After that, patch 4/7 checks
> stack requirement for probed instructions. Patches 5/7 - 7/7 are similar
> to patch v5, except:
>
> 1. As Tixy proposed, unoptimized probes are also suffer from stack
> problem (https://lkml.org/lkml/2014/9/1/548 ). Commit d30a0c8b saves
> 64 bytes for them, but for instruction use register addressing (like
> 'str r0, [sp, r1]'), 64 bytes are unsafe. Patch 5/7 prohibit such
> probing according to stack information collected by checker.
By the way, this sounds like a bugfix rather than an improvement.
Is it possible to separate 1/7-5/7 as a bugfix series? I think those
should go to 3.18.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
On Fri, 2014-10-24 at 09:52 +0900, Masami Hiramatsu wrote:
> (2014/10/22 20:31), Wang Nan wrote:
> > Previous 5 version of ARM OPTPROBES patches are unable to deal with
> > stack storing instructions correctly. V5 patches disallow optimizing
> > every protential stack store instructions based on pessimistic
> > assumption. Which, as Tixy comments, 'excludes the main use of
> > kprobes'. (https://lkml.org/lkml/2014/8/29/117 )
> >
> > The main obstacle which prevents us from computing stack requirement is
> > the missing of per-instruction decoder in probes_decode_insn() and its
> > friends. Only part of instructions have their decoders (and not in
> > each case).
> >
> > In this patch series, I propose 'checker', which allows us define
> > functions for each type of instruction, extract more information. Stack
> > consumption computing is an example. Checker can be further employed to
> > determine whether one instruction is possible to execute directy in
> > optimized kprobe. I'd like to expand current checker framework by
> > chaining checkers together. After that, I believe most of ARM
> > instructions can be executed directly like x86, kprobe performace can be
> > improved.
> >
> > The first 3 patches introduces checker. After that, patch 4/7 checks
> > stack requirement for probed instructions. Patches 5/7 - 7/7 are similar
> > to patch v5, except:
> >
> > 1. As Tixy proposed, unoptimized probes are also suffer from stack
> > problem (https://lkml.org/lkml/2014/9/1/548 ). Commit d30a0c8b saves
> > 64 bytes for them, but for instruction use register addressing (like
> > 'str r0, [sp, r1]'), 64 bytes are unsafe. Patch 5/7 prohibit such
> > probing according to stack information collected by checker.
>
> By the way, this sounds like a bugfix rather than an improvement.
> Is it possible to separate 1/7-5/7 as a bugfix series? I think those
> should go to 3.18.
I believe that problem has existed since kprobes was first implemented
on ARM 7 years ago, and the problematic instruction type doesn't appear
to get generated by GCC so, in my opinion, I don't think there is any
particular urgency to fix this as a bug in the current and, by
implication, stable kernels.
--
Tixy
On 2014/10/24 17:02, Jon Medhurst (Tixy) wrote:
> On Fri, 2014-10-24 at 09:52 +0900, Masami Hiramatsu wrote:
>> (2014/10/22 20:31), Wang Nan wrote:
>>> Previous 5 version of ARM OPTPROBES patches are unable to deal with
>>> stack storing instructions correctly. V5 patches disallow optimizing
>>> every protential stack store instructions based on pessimistic
>>> assumption. Which, as Tixy comments, 'excludes the main use of
>>> kprobes'. (https://lkml.org/lkml/2014/8/29/117 )
>>>
>>> The main obstacle which prevents us from computing stack requirement is
>>> the missing of per-instruction decoder in probes_decode_insn() and its
>>> friends. Only part of instructions have their decoders (and not in
>>> each case).
>>>
>>> In this patch series, I propose 'checker', which allows us define
>>> functions for each type of instruction, extract more information. Stack
>>> consumption computing is an example. Checker can be further employed to
>>> determine whether one instruction is possible to execute directy in
>>> optimized kprobe. I'd like to expand current checker framework by
>>> chaining checkers together. After that, I believe most of ARM
>>> instructions can be executed directly like x86, kprobe performace can be
>>> improved.
>>>
>>> The first 3 patches introduces checker. After that, patch 4/7 checks
>>> stack requirement for probed instructions. Patches 5/7 - 7/7 are similar
>>> to patch v5, except:
>>>
>>> 1. As Tixy proposed, unoptimized probes are also suffer from stack
>>> problem (https://lkml.org/lkml/2014/9/1/548 ). Commit d30a0c8b saves
>>> 64 bytes for them, but for instruction use register addressing (like
>>> 'str r0, [sp, r1]'), 64 bytes are unsafe. Patch 5/7 prohibit such
>>> probing according to stack information collected by checker.
>>
>> By the way, this sounds like a bugfix rather than an improvement.
>> Is it possible to separate 1/7-5/7 as a bugfix series? I think those
>> should go to 3.18.
>
> I believe that problem has existed since kprobes was first implemented
> on ARM 7 years ago, and the problematic instruction type doesn't appear
> to get generated by GCC so, in my opinion, I don't think there is any
> particular urgency to fix this as a bug in the current and, by
> implication, stable kernels.
>
Anyway, I have done the separation. Please refer to:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/297031.html
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/297036.html
There is a big change in checker code in the first thread. Please help me review
whether checker is acceptable. The next thing I'll do is to create another checker
table to check whether the probed insn can be directly executed as in x86_64.
Thanks.
On Sat, 2014-10-25 at 17:49 +0800, Wang Nan wrote:
[...]
> Anyway, I have done the separation. Please refer to:
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/297031.html
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/297036.html
>
> There is a big change in checker code in the first thread. Please help me review
> whether checker is acceptable.
I had started reviewing the previous version, I'll switch to the new
one.
I do prefer the new checker code better, and the only obvious
alternative I can think off would be to break down the decoding tables
into a lot more special cases of instruction forms, which I isn't as
scalable, so I don't like that idea.
However, I wonder if there is scope for the checker functions to
recursively call probes_decode_insn rather than decoding instructions in
C code. I don't know if that would be clearer and/or smaller or not.
Below is something I've just thrown together to get a feel of how it
could look. The decode table could possibly incorporate patterns to
cover instructions types that you split up in PATCH 1, e.g. so we might
not need separate PROBES_STORE and PROBES_STORE_EXTRA (depends if that
ends up making things simpler or not)...
int stack_use_none(probes_opcode_t insn,
struct arch_probes_insn *asi,
const struct decode_header *h)
{
asi->stack_space = 0;
return INSN_GOOD_NO_SLOT;
}
int stack_use_unknown(probes_opcode_t insn,
struct arch_probes_insn *asi,
const struct decode_header *h)
{
asi->stack_space = -1;
return INSN_GOOD_NO_SLOT;
}
int stack_use_imm_x0x(probes_opcode_t insn,
struct arch_probes_insn *asi,
const struct decode_header *h)
{
asi->stack_space = ((insn & 0xf00) >> 4) + (insn & 0xf);
return INSN_GOOD_NO_SLOT;
}
int stack_use_imm_xxx(probes_opcode_t insn,
struct arch_probes_insn *asi,
const struct decode_header *h)
{
asi->stack_space = insn & 0xfff;
return INSN_GOOD_NO_SLOT;
}
enum {
STACK_USE_NONE,
STACK_USE_UNKNOWN,
STACK_USE_FIXED_X0X,
STACK_USE_FIXED_XXX,
NUM_STACK_USE_TYPES
};
static const union decode_action chk_stack_actions[] = {
[STACK_USE_NONE] = {.handler = stack_use_none},
[STACK_USE_UNKNOWN] = {.handler = stack_use_unknown},
[STACK_USE_FIXED_X0X] = {.handler = stack_use_imm_x0x}
[STACK_USE_FIXED_XXX] = {.handler = stack_use_imm_xxx}
}
enum probes_insn __kprobes chk_stack_use_arm(probes_opcode_t insn,
struct arch_probes_insn *asi,
const struct decode_header *h)
{
static const union decode_item table[] = {
/* Register indexed addressing modes with SP as index register (!)*/
DECODE_OR (0x0040000d, 0x0000000d),
/* Register indexed addressing modes with SP as base register */
DECODE_CUSTOM (0x004f0000, 0x000d0000, STACK_USE_UNKOWN,
REGS(0, 0, 0, 0, 0)),
/* STR{,B} with immediate pre-indexed addressing mode with SP base address */
DECODE_CUSTOM (0x05cf0000, 0x054d0000, STACK_USE_FIXED_XXX,
REGS(0, 0, 0, 0, 0)),
/* STR{H,D} with immediate pre-indexed addressing mode with SP base address */
DECODE_CUSTOM (0x05cf0000, 0x014d0000, STACK_USE_FIXED_X0X,
REGS(0, 0, 0, 0, 0)),
... other rules here, possibly including ...
... REGS values like 'NOSP' to reject certain forms ...
/* Catch all remaining cases */
DECODE_CUSTOM (0, 0, STACK_USE_NONE, REGS(0, 0, 0, 0, 0))
}
return probes_decode_insn(insn, asi, table, false, false, chk_stack_actions, NULL);
}
And in the dubious case that anyone wants to copy any of the above, it's
Signed-off-by: Jon Medhurst <[email protected]>
--
Tixy
On Mon, 2014-10-27 at 17:17 +0000, Jon Medhurst (Tixy) wrote:
[...]
> The decode table could possibly incorporate patterns to
> cover instructions types that you split up in PATCH 1, e.g. so we
> might not need separate PROBES_STORE and PROBES_STORE_EXTRA (
Sorry, I got that a bit wrong, the first patch only splits loads and
stores and doesn't create create any new 'extra' instruction types.
However, my comment could still apply to that split between between
loads and stores; for many of them, the difference is just a single bit
that is possibly cheap or free to test in the checkers.
The reason I am thinking along these lines is that each additional enum
value in the instruction types adds an entry into every action and
checker table, as well as expanding the decoding tables to detect them.
So I just want to make sure that we think these additions result in a
net benefit in code size and complexity.
--
Tixy