| Message ID | 20200406080936.7180-9-jniethe5@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Initial Prefixed Instruction support | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/merge (2c0ce4ff35994a7b12cc9879ced52c9e7c2e6667) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/next (6ba4a2d3591039aea1cb45c7c42262d26351a2fa) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch linus/master (a10c9c710f9ecea87b9f4bbb837467893b4bef01) |
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/fixes (1d0c32ec3b860a32df593a22bad0d1dbc5546a59) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 1 warnings, 1 checks, 138 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote: > In preparation for an instruction data type that can not be directly > used with the '==' operator use functions for checking equality. LGTM except one comment below, otherwise Reviewed-by: Balamuruhan S <bala24@linux.ibm.com> > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > --- > v5: Remove ppc_inst_null() > --- > arch/powerpc/include/asm/inst.h | 5 +++++ > arch/powerpc/kernel/trace/ftrace.c | 15 ++++++++------- > arch/powerpc/lib/code-patching.c | 12 ++++++------ > arch/powerpc/lib/test_emulate_step.c | 2 +- > arch/powerpc/xmon/xmon.c | 4 ++-- > 5 files changed, 22 insertions(+), 16 deletions(-) > > diff --git a/arch/powerpc/include/asm/inst.h > b/arch/powerpc/include/asm/inst.h > index 78eb1481f1f6..54ee46b0a7c9 100644 > --- a/arch/powerpc/include/asm/inst.h > +++ b/arch/powerpc/include/asm/inst.h > @@ -23,4 +23,9 @@ static inline u32 ppc_inst_swab(u32 x) > return ppc_inst(swab32(ppc_inst_val(x))); > } > > +static inline bool ppc_inst_equal(u32 x, u32 y) > +{ > + return x == y; > +} > + > #endif /* _ASM_INST_H */ > diff --git a/arch/powerpc/kernel/trace/ftrace.c > b/arch/powerpc/kernel/trace/ftrace.c > index 62ff429bddc4..784b5746cc55 100644 > --- a/arch/powerpc/kernel/trace/ftrace.c > +++ b/arch/powerpc/kernel/trace/ftrace.c > @@ -72,7 +72,7 @@ ftrace_modify_code(unsigned long ip, unsigned int old, > unsigned int new) > return -EFAULT; > > /* Make sure it is what we expect it to be */ > - if (replaced != old) { > + if (!ppc_inst_equal(replaced, old)) { > pr_err("%p: replaced (%#x) != old (%#x)", > (void *)ip, ppc_inst_val(replaced), ppc_inst_val(old)); > return -EINVAL; > @@ -170,7 +170,8 @@ __ftrace_make_nop(struct module *mod, > } > > /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */ > - if (op != ppc_inst(PPC_INST_MFLR) && op != ppc_inst(PPC_INST_STD_LR)) { > + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_MFLR)) && > + !ppc_inst_equal(op, ppc_inst(PPC_INST_STD_LR))) { > pr_err("Unexpected instruction %08x around bl _mcount\n", > ppc_inst_val(op)); > return -EINVAL; > } > @@ -200,7 +201,7 @@ __ftrace_make_nop(struct module *mod, > return -EFAULT; > } > > - if (op != ppc_inst(PPC_INST_LD_TOC)) { > + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_LD_TOC))) { > pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC, > ppc_inst_val(op)); > return -EINVAL; > } > @@ -497,7 +498,7 @@ expected_nop_sequence(void *ip, unsigned int op0, > unsigned int op1) > * The load offset is different depending on the ABI. For simplicity > * just mask it out when doing the compare. > */ > - if ((op0 != ppc_inst(0x48000008)) || (ppc_inst_val(op1) & 0xffff0000) > != 0xe8410000) > + if ((!ppc_inst_equal(op0), ppc_inst(0x48000008)) || (ppc_inst_val(op1) > & 0xffff0000) != 0xe8410000) > return 0; > return 1; > } > @@ -506,7 +507,7 @@ static int > expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1) > { > /* look for patched "NOP" on ppc64 with -mprofile-kernel */ > - if (op0 != ppc_inst(PPC_INST_NOP)) > + if (!ppc_inst_equal(op0, ppc_inst(PPC_INST_NOP))) > return 0; > return 1; > } > @@ -589,7 +590,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long > addr) > return -EFAULT; > > /* It should be pointing to a nop */ > - if (op != ppc_inst(PPC_INST_NOP)) { > + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_NOP))) { > pr_err("Expected NOP but have %x\n", op); > return -EINVAL; > } > @@ -646,7 +647,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace > *rec, unsigned long addr) > return -EFAULT; > } > > - if (op != ppc_inst(PPC_INST_NOP)) { > + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_NOP))) { > pr_err("Unexpected call sequence at %p: %x\n", ip, > ppc_inst_val(op)); > return -EINVAL; > } > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code- > patching.c > index 3f88d2a4400c..33654c6334a9 100644 > --- a/arch/powerpc/lib/code-patching.c > +++ b/arch/powerpc/lib/code-patching.c > @@ -478,7 +478,7 @@ static void __init test_branch_iform(void) > /* Check flags are masked correctly */ > err = create_branch(&instr, &instr, addr, 0xFFFFFFFC); > check(instr_is_branch_to_addr(&instr, addr)); > - check(instr == ppc_inst(0x48000000)); > + check(ppc_inst_equal(instr, ppc_inst(0x48000000))); > } > > static void __init test_create_function_call(void) > @@ -563,7 +563,7 @@ static void __init test_branch_bform(void) > /* Check flags are masked correctly */ > err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC); > check(instr_is_branch_to_addr(&instr, addr)); > - check(instr == ppc_inst(0x43FF0000)); > + check(ppc_inst_equal(instr, ppc_inst(0x43FF0000))); > } > > static void __init test_translate_branch(void) > @@ -597,7 +597,7 @@ static void __init test_translate_branch(void) > patch_instruction(q, instr); > check(instr_is_branch_to_addr(p, addr)); > check(instr_is_branch_to_addr(q, addr)); > - check(*q == ppc_inst(0x4a000000)); > + check(ppc_inst_equal(*q, ppc_inst(0x4a000000))); > > /* Maximum positive case, move x to x - 32 MB + 4 */ > p = buf + 0x2000000; > @@ -608,7 +608,7 @@ static void __init test_translate_branch(void) > patch_instruction(q, instr); > check(instr_is_branch_to_addr(p, addr)); > check(instr_is_branch_to_addr(q, addr)); > - check(*q == ppc_inst(0x49fffffc)); > + check(ppc_inst_equal(*q, ppc_inst(0x49fffffc))); > > /* Jump to x + 16 MB moved to x + 20 MB */ > p = buf; > @@ -654,7 +654,7 @@ static void __init test_translate_branch(void) > patch_instruction(q, instr); > check(instr_is_branch_to_addr(p, addr)); > check(instr_is_branch_to_addr(q, addr)); > - check(*q == ppc_inst(0x43ff8000)); > + check(ppc_inst_equal(*q, ppc_inst(0x43ff8000))); > > /* Maximum positive case, move x to x - 32 KB + 4 */ > p = buf + 0x8000; > @@ -666,7 +666,7 @@ static void __init test_translate_branch(void) > patch_instruction(q, instr); > check(instr_is_branch_to_addr(p, addr)); > check(instr_is_branch_to_addr(q, addr)); > - check(*q == ppc_inst(0x43ff7ffc)); > + check(ppc_inst_equal(*q, ppc_inst(0x43ff7ffc))); > > /* Jump to x + 12 KB moved to x + 20 KB */ > p = buf; > diff --git a/arch/powerpc/lib/test_emulate_step.c > b/arch/powerpc/lib/test_emulate_step.c > index 60f7eb24d742..16387a9bfda0 100644 > --- a/arch/powerpc/lib/test_emulate_step.c > +++ b/arch/powerpc/lib/test_emulate_step.c > @@ -865,7 +865,7 @@ static int __init execute_compute_instr(struct pt_regs > *regs, > extern int exec_instr(struct pt_regs *regs); > extern s32 patch__exec_instr; > > - if (!regs || !instr) > + if (!regs || !ppc_inst_val(instr)) This change should go in to below patch, [PATCH v5 06/21] powerpc: Use an accessor for instructions -- Bala > return -EINVAL; > > /* Patch the NOP with the actual instruction */ > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index 3c1fb46bfacf..f6c87d3d53ea 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -950,7 +950,7 @@ static void remove_bpts(void) > if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP) > continue; > if (mread(bp->address, &instr, 4) == 4 > - && instr == ppc_inst(bpinstr) > + && ppc_inst_equal(instr, ppc_inst(bpinstr)) > && patch_instruction( > (unsigned int *)bp->address, bp->instr[0]) != 0) > printf("Couldn't remove breakpoint at %lx\n", > @@ -2860,7 +2860,7 @@ generic_inst_dump(unsigned long adr, long count, int > praddr, > break; > } > inst = ppc_inst(GETWORD(val)); > - if (adr > first_adr && inst == last_inst) { > + if (adr > first_adr && ppc_inst_equal(inst, last_inst)) { > if (!dotted) { > printf(" ...\n"); > dotted = 1;
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h index 78eb1481f1f6..54ee46b0a7c9 100644 --- a/arch/powerpc/include/asm/inst.h +++ b/arch/powerpc/include/asm/inst.h @@ -23,4 +23,9 @@ static inline u32 ppc_inst_swab(u32 x) return ppc_inst(swab32(ppc_inst_val(x))); } +static inline bool ppc_inst_equal(u32 x, u32 y) +{ + return x == y; +} + #endif /* _ASM_INST_H */ diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c index 62ff429bddc4..784b5746cc55 100644 --- a/arch/powerpc/kernel/trace/ftrace.c +++ b/arch/powerpc/kernel/trace/ftrace.c @@ -72,7 +72,7 @@ ftrace_modify_code(unsigned long ip, unsigned int old, unsigned int new) return -EFAULT; /* Make sure it is what we expect it to be */ - if (replaced != old) { + if (!ppc_inst_equal(replaced, old)) { pr_err("%p: replaced (%#x) != old (%#x)", (void *)ip, ppc_inst_val(replaced), ppc_inst_val(old)); return -EINVAL; @@ -170,7 +170,8 @@ __ftrace_make_nop(struct module *mod, } /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */ - if (op != ppc_inst(PPC_INST_MFLR) && op != ppc_inst(PPC_INST_STD_LR)) { + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_MFLR)) && + !ppc_inst_equal(op, ppc_inst(PPC_INST_STD_LR))) { pr_err("Unexpected instruction %08x around bl _mcount\n", ppc_inst_val(op)); return -EINVAL; } @@ -200,7 +201,7 @@ __ftrace_make_nop(struct module *mod, return -EFAULT; } - if (op != ppc_inst(PPC_INST_LD_TOC)) { + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_LD_TOC))) { pr_err("Expected %08x found %08x\n", PPC_INST_LD_TOC, ppc_inst_val(op)); return -EINVAL; } @@ -497,7 +498,7 @@ expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1) * The load offset is different depending on the ABI. For simplicity * just mask it out when doing the compare. */ - if ((op0 != ppc_inst(0x48000008)) || (ppc_inst_val(op1) & 0xffff0000) != 0xe8410000) + if ((!ppc_inst_equal(op0), ppc_inst(0x48000008)) || (ppc_inst_val(op1) & 0xffff0000) != 0xe8410000) return 0; return 1; } @@ -506,7 +507,7 @@ static int expected_nop_sequence(void *ip, unsigned int op0, unsigned int op1) { /* look for patched "NOP" on ppc64 with -mprofile-kernel */ - if (op0 != ppc_inst(PPC_INST_NOP)) + if (!ppc_inst_equal(op0, ppc_inst(PPC_INST_NOP))) return 0; return 1; } @@ -589,7 +590,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) return -EFAULT; /* It should be pointing to a nop */ - if (op != ppc_inst(PPC_INST_NOP)) { + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_NOP))) { pr_err("Expected NOP but have %x\n", op); return -EINVAL; } @@ -646,7 +647,7 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr) return -EFAULT; } - if (op != ppc_inst(PPC_INST_NOP)) { + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_NOP))) { pr_err("Unexpected call sequence at %p: %x\n", ip, ppc_inst_val(op)); return -EINVAL; } diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c index 3f88d2a4400c..33654c6334a9 100644 --- a/arch/powerpc/lib/code-patching.c +++ b/arch/powerpc/lib/code-patching.c @@ -478,7 +478,7 @@ static void __init test_branch_iform(void) /* Check flags are masked correctly */ err = create_branch(&instr, &instr, addr, 0xFFFFFFFC); check(instr_is_branch_to_addr(&instr, addr)); - check(instr == ppc_inst(0x48000000)); + check(ppc_inst_equal(instr, ppc_inst(0x48000000))); } static void __init test_create_function_call(void) @@ -563,7 +563,7 @@ static void __init test_branch_bform(void) /* Check flags are masked correctly */ err = create_cond_branch(&instr, iptr, addr, 0xFFFFFFFC); check(instr_is_branch_to_addr(&instr, addr)); - check(instr == ppc_inst(0x43FF0000)); + check(ppc_inst_equal(instr, ppc_inst(0x43FF0000))); } static void __init test_translate_branch(void) @@ -597,7 +597,7 @@ static void __init test_translate_branch(void) patch_instruction(q, instr); check(instr_is_branch_to_addr(p, addr)); check(instr_is_branch_to_addr(q, addr)); - check(*q == ppc_inst(0x4a000000)); + check(ppc_inst_equal(*q, ppc_inst(0x4a000000))); /* Maximum positive case, move x to x - 32 MB + 4 */ p = buf + 0x2000000; @@ -608,7 +608,7 @@ static void __init test_translate_branch(void) patch_instruction(q, instr); check(instr_is_branch_to_addr(p, addr)); check(instr_is_branch_to_addr(q, addr)); - check(*q == ppc_inst(0x49fffffc)); + check(ppc_inst_equal(*q, ppc_inst(0x49fffffc))); /* Jump to x + 16 MB moved to x + 20 MB */ p = buf; @@ -654,7 +654,7 @@ static void __init test_translate_branch(void) patch_instruction(q, instr); check(instr_is_branch_to_addr(p, addr)); check(instr_is_branch_to_addr(q, addr)); - check(*q == ppc_inst(0x43ff8000)); + check(ppc_inst_equal(*q, ppc_inst(0x43ff8000))); /* Maximum positive case, move x to x - 32 KB + 4 */ p = buf + 0x8000; @@ -666,7 +666,7 @@ static void __init test_translate_branch(void) patch_instruction(q, instr); check(instr_is_branch_to_addr(p, addr)); check(instr_is_branch_to_addr(q, addr)); - check(*q == ppc_inst(0x43ff7ffc)); + check(ppc_inst_equal(*q, ppc_inst(0x43ff7ffc))); /* Jump to x + 12 KB moved to x + 20 KB */ p = buf; diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c index 60f7eb24d742..16387a9bfda0 100644 --- a/arch/powerpc/lib/test_emulate_step.c +++ b/arch/powerpc/lib/test_emulate_step.c @@ -865,7 +865,7 @@ static int __init execute_compute_instr(struct pt_regs *regs, extern int exec_instr(struct pt_regs *regs); extern s32 patch__exec_instr; - if (!regs || !instr) + if (!regs || !ppc_inst_val(instr)) return -EINVAL; /* Patch the NOP with the actual instruction */ diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index 3c1fb46bfacf..f6c87d3d53ea 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -950,7 +950,7 @@ static void remove_bpts(void) if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP) continue; if (mread(bp->address, &instr, 4) == 4 - && instr == ppc_inst(bpinstr) + && ppc_inst_equal(instr, ppc_inst(bpinstr)) && patch_instruction( (unsigned int *)bp->address, bp->instr[0]) != 0) printf("Couldn't remove breakpoint at %lx\n", @@ -2860,7 +2860,7 @@ generic_inst_dump(unsigned long adr, long count, int praddr, break; } inst = ppc_inst(GETWORD(val)); - if (adr > first_adr && inst == last_inst) { + if (adr > first_adr && ppc_inst_equal(inst, last_inst)) { if (!dotted) { printf(" ...\n"); dotted = 1;
In preparation for an instruction data type that can not be directly used with the '==' operator use functions for checking equality. Signed-off-by: Jordan Niethe <jniethe5@gmail.com> --- v5: Remove ppc_inst_null() --- arch/powerpc/include/asm/inst.h | 5 +++++ arch/powerpc/kernel/trace/ftrace.c | 15 ++++++++------- arch/powerpc/lib/code-patching.c | 12 ++++++------ arch/powerpc/lib/test_emulate_step.c | 2 +- arch/powerpc/xmon/xmon.c | 4 ++-- 5 files changed, 22 insertions(+), 16 deletions(-)