From: Balamuruhan S <bala24@linux.ibm.com>
To: Jordan Niethe <jniethe5@gmail.com>, linuxppc-dev@lists.ozlabs.org
Cc: alistair@popple.id.au, npiggin@gmail.com, dja@axtens.net
Subject: Re: [PATCH v5 08/21] powerpc: Introduce functions for instruction equality
Date: Tue, 07 Apr 2020 13:07:42 +0530 [thread overview]
Message-ID: <1a99be1860ca76bf48a6eac8687488b6c41d2c87.camel@linux.ibm.com> (raw)
In-Reply-To: <20200406080936.7180-9-jniethe5@gmail.com>
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;
next prev parent reply other threads:[~2020-04-07 7:39 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-06 8:09 [PATCH v5 00/21] Initial Prefixed Instruction support Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 01/21] powerpc/xmon: Remove store_inst() for patch_instruction() Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 02/21] powerpc/xmon: Move out-of-line instructions to text section Jordan Niethe
2020-04-07 6:45 ` Balamuruhan S
2020-04-09 6:11 ` Christophe Leroy
2020-04-09 7:26 ` Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 03/21] powerpc: Change calling convention for create_branch() et. al Jordan Niethe
2020-04-06 10:25 ` kbuild test robot
2020-04-06 10:25 ` kbuild test robot
2020-04-07 6:10 ` Balamuruhan S
2020-04-07 6:35 ` Jordan Niethe
2020-04-07 6:59 ` Balamuruhan S
2020-04-06 8:09 ` [PATCH v5 04/21] powerpc: Use a macro for creating instructions from u32s Jordan Niethe
2020-04-07 6:40 ` Balamuruhan S
2020-04-07 8:27 ` Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 05/21] powerpc: Use a function for getting the instruction op code Jordan Niethe
2020-04-06 8:22 ` Christophe Leroy
2020-04-06 9:38 ` Jordan Niethe
2020-04-07 7:04 ` Balamuruhan S
2020-04-07 8:32 ` Jordan Niethe
2020-04-08 18:21 ` Segher Boessenkool
2020-04-09 4:48 ` Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 06/21] powerpc: Use an accessor for instructions Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 07/21] powerpc: Use a function for byte swapping instructions Jordan Niethe
2020-04-07 7:42 ` Balamuruhan S
2020-04-06 8:09 ` [PATCH v5 08/21] powerpc: Introduce functions for instruction equality Jordan Niethe
2020-04-07 7:37 ` Balamuruhan S [this message]
2020-04-06 8:09 ` [PATCH v5 09/21] powerpc: Use a datatype for instructions Jordan Niethe
2020-04-06 10:34 ` kbuild test robot
2020-04-06 10:34 ` kbuild test robot
2020-04-06 10:35 ` kbuild test robot
2020-04-06 10:35 ` kbuild test robot
2020-04-07 10:30 ` Balamuruhan S
2020-04-08 2:11 ` Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 10/21] powerpc: Use a function for reading instructions Jordan Niethe
2020-04-07 10:42 ` Balamuruhan S
2020-04-06 8:09 ` [PATCH v5 11/21] powerpc: Define and use __get_user_instr{, inatomic}() Jordan Niethe
2020-04-07 10:48 ` Balamuruhan S
2020-04-08 2:13 ` Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 12/21] powerpc: Introduce a function for reporting instruction length Jordan Niethe
2020-04-07 11:14 ` Balamuruhan S
2020-04-08 2:14 ` Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 13/21] powerpc/xmon: Use a function for reading instructions Jordan Niethe
2020-04-07 11:30 ` Balamuruhan S
2020-04-08 2:18 ` Jordan Niethe
2020-04-09 5:04 ` Balamuruhan S
2020-04-09 5:14 ` Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 14/21] powerpc/xmon: Move insertion of breakpoint for xol'ing Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 15/21] powerpc: Make test_translate_branch() independent of instruction length Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 16/21] powerpc: Enable Prefixed Instructions Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 17/21] powerpc: Define new SRR1 bits for a future ISA version Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type Jordan Niethe
2020-04-06 9:52 ` Alistair Popple
2020-04-06 10:25 ` Christophe Leroy
2020-04-06 11:13 ` Jordan Niethe
2020-04-08 18:11 ` Segher Boessenkool
2020-04-08 18:43 ` Christophe Leroy
2020-04-06 10:42 ` Jordan Niethe
2020-04-07 1:39 ` Alistair Popple
2020-04-06 11:04 ` kbuild test robot
2020-04-06 11:04 ` kbuild test robot
2020-04-13 12:04 ` Balamuruhan S
2020-04-15 4:40 ` Jordan Niethe
2020-04-15 8:14 ` Balamuruhan S
2020-04-06 8:09 ` [PATCH v5 19/21] powerpc: Support prefixed instructions in alignment handler Jordan Niethe
2020-04-06 8:09 ` [PATCH v5 20/21] powerpc sstep: Add support for prefixed load/stores Jordan Niethe
2020-04-06 11:29 ` kbuild test robot
2020-04-06 11:29 ` kbuild test robot
2020-04-06 8:09 ` [PATCH v5 21/21] powerpc sstep: Add support for prefixed fixed-point arithmetic Jordan Niethe
2020-04-09 6:39 ` [PATCH v5 00/21] Initial Prefixed Instruction support Christophe Leroy
2020-04-09 7:28 ` Jordan Niethe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1a99be1860ca76bf48a6eac8687488b6c41d2c87.camel@linux.ibm.com \
--to=bala24@linux.ibm.com \
--cc=alistair@popple.id.au \
--cc=dja@axtens.net \
--cc=jniethe5@gmail.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=npiggin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.