Re: [PATCH v5 03/21] powerpc: Change calling convention for create_branch() et. al.
On Tue, 2020-04-07 at 16:35 +1000, Jordan Niethe wrote:
> On Tue, Apr 7, 2020 at 4:10 PM Balamuruhan S wrote:
> > On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > > create_branch(), create_cond_branch() and translate_branch() return the
> > > instruction that they create, or return 0 to signal an error. Seperate
> >
> > s/seperate/separate
> thanks.
> > > these concerns in preparation for an instruction type that is not just
> > > an unsigned int. Fill the created instruction to a pointer passed as
> > > the first parameter to the function and use a non-zero return value to
> > > signify an error.
> > >
> > > Signed-off-by: Jordan Niethe
> > > ---
> > > v5: New to series
> > > ---
> > > arch/powerpc/include/asm/code-patching.h | 12 +-
> > > arch/powerpc/kernel/optprobes.c | 24 ++--
> > > arch/powerpc/kernel/setup_32.c | 2 +-
> > > arch/powerpc/kernel/trace/ftrace.c | 24 ++--
> > > arch/powerpc/lib/code-patching.c | 133 +--
> > > arch/powerpc/lib/feature-fixups.c| 5 +-
> > > 6 files changed, 117 insertions(+), 83 deletions(-)
> > >
> > > diff --git a/arch/powerpc/include/asm/code-patching.h
> > > b/arch/powerpc/include/asm/code-patching.h
> > > index 898b54262881..351dda7215b6 100644
> > > --- a/arch/powerpc/include/asm/code-patching.h
> > > +++ b/arch/powerpc/include/asm/code-patching.h
> > > @@ -22,10 +22,10 @@
> > > #define BRANCH_ABSOLUTE 0x2
> > >
> > > bool is_offset_in_branch_range(long offset);
> > > -unsigned int create_branch(const unsigned int *addr,
> > > -unsigned long target, int flags);
> > > -unsigned int create_cond_branch(const unsigned int *addr,
> > > - unsigned long target, int flags);
> > > +int create_branch(unsigned int *instr, const unsigned int *addr,
> > > + unsigned long target, int flags);
> > > +int create_cond_branch(unsigned int *instr, const unsigned int *addr,
> > > +unsigned long target, int flags);
> > > int patch_branch(unsigned int *addr, unsigned long target, int flags);
> > > int patch_instruction(unsigned int *addr, unsigned int instr);
> > > int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> > > @@ -60,8 +60,8 @@ int instr_is_relative_branch(unsigned int instr);
> > > int instr_is_relative_link_branch(unsigned int instr);
> > > int instr_is_branch_to_addr(const unsigned int *instr, unsigned long
> > > addr);
> > > unsigned long branch_target(const unsigned int *instr);
> > > -unsigned int translate_branch(const unsigned int *dest,
> > > - const unsigned int *src);
> > > +int translate_branch(unsigned int *instr, const unsigned int *dest,
> > > + const unsigned int *src);
> > > extern bool is_conditional_branch(unsigned int instr);
> > > #ifdef CONFIG_PPC_BOOK3E_64
> > > void __patch_exception(int exc, unsigned long addr);
> > > diff --git a/arch/powerpc/kernel/optprobes.c
> > > b/arch/powerpc/kernel/optprobes.c
> > > index 024f7aad1952..445b3dad82dc 100644
> > > --- a/arch/powerpc/kernel/optprobes.c
> > > +++ b/arch/powerpc/kernel/optprobes.c
> > > @@ -251,15 +251,17 @@ int arch_prepare_optimized_kprobe(struct
> > > optimized_kprobe *op, struct kprobe *p)
> > > goto error;
> > > }
> > >
> > > - branch_op_callback = create_branch((unsigned int *)buff +
> > > TMPL_CALL_HDLR_IDX,
> > > - (unsigned long)op_callback_addr,
> > > - BRANCH_SET_LINK);
> > > + rc = create_branch(_op_callback,
> > > +(unsigned int *)buff + TMPL_CALL_HDLR_IDX,
> > > +(unsigned long)op_callback_addr,
> > > +BRANCH_SET_LINK);
> > >
> > > - branch_emulate_step = create_branch((unsigned int *)buff +
> > > TMPL_EMULATE_IDX,
> > > - (unsigned long)emulate_step_addr,
> > > - BRANCH_SET_LINK);
> > > + rc |= create_branch(_emulate_step,
> > > + (unsigned int *)buff + TMPL_EMULATE_IDX,
> > > + (unsigned long)emulate_step_addr,
> > > + BRANCH_SET_LINK);
> > >
> > > - if (!branch_op_callback || !branch_emulate_step)
> > > + if (rc)
> > > goto error;
> > >
> > > patch_instruction(buff + TMPL_CALL_HDLR_IDX, branch_op_callback);
> > > @@ -305,6 +307,7 @@ int arch_check_optimized_kprobe(struct
> > > optimized_kprobe
> > > *op)
> > >
> > > void arch_optimize_kprobes(struct list_head *oplist)
> > > {
> > > + unsigned int instr;
> > > struct optimized_kprobe *op;
> > > struct optimized_kprobe *tmp;
> > >
> > > @@ -315,9 +318,10 @@ void arch_optimize_kprobes(struct list_head *oplist)
> > >*/
> > > memcpy(op->optinsn.copied_insn, op->kp.addr,
> > >
Re: [PATCH v5 03/21] powerpc: Change calling convention for create_branch() et. al.
On Tue, Apr 7, 2020 at 4:10 PM Balamuruhan S wrote:
>
> On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> > create_branch(), create_cond_branch() and translate_branch() return the
> > instruction that they create, or return 0 to signal an error. Seperate
>
> s/seperate/separate
thanks.
>
> > these concerns in preparation for an instruction type that is not just
> > an unsigned int. Fill the created instruction to a pointer passed as
> > the first parameter to the function and use a non-zero return value to
> > signify an error.
> >
> > Signed-off-by: Jordan Niethe
> > ---
> > v5: New to series
> > ---
> > arch/powerpc/include/asm/code-patching.h | 12 +-
> > arch/powerpc/kernel/optprobes.c | 24 ++--
> > arch/powerpc/kernel/setup_32.c | 2 +-
> > arch/powerpc/kernel/trace/ftrace.c | 24 ++--
> > arch/powerpc/lib/code-patching.c | 133 +--
> > arch/powerpc/lib/feature-fixups.c| 5 +-
> > 6 files changed, 117 insertions(+), 83 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/code-patching.h
> > b/arch/powerpc/include/asm/code-patching.h
> > index 898b54262881..351dda7215b6 100644
> > --- a/arch/powerpc/include/asm/code-patching.h
> > +++ b/arch/powerpc/include/asm/code-patching.h
> > @@ -22,10 +22,10 @@
> > #define BRANCH_ABSOLUTE 0x2
> >
> > bool is_offset_in_branch_range(long offset);
> > -unsigned int create_branch(const unsigned int *addr,
> > -unsigned long target, int flags);
> > -unsigned int create_cond_branch(const unsigned int *addr,
> > - unsigned long target, int flags);
> > +int create_branch(unsigned int *instr, const unsigned int *addr,
> > + unsigned long target, int flags);
> > +int create_cond_branch(unsigned int *instr, const unsigned int *addr,
> > +unsigned long target, int flags);
> > int patch_branch(unsigned int *addr, unsigned long target, int flags);
> > int patch_instruction(unsigned int *addr, unsigned int instr);
> > int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> > @@ -60,8 +60,8 @@ int instr_is_relative_branch(unsigned int instr);
> > int instr_is_relative_link_branch(unsigned int instr);
> > int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
> > unsigned long branch_target(const unsigned int *instr);
> > -unsigned int translate_branch(const unsigned int *dest,
> > - const unsigned int *src);
> > +int translate_branch(unsigned int *instr, const unsigned int *dest,
> > + const unsigned int *src);
> > extern bool is_conditional_branch(unsigned int instr);
> > #ifdef CONFIG_PPC_BOOK3E_64
> > void __patch_exception(int exc, unsigned long addr);
> > diff --git a/arch/powerpc/kernel/optprobes.c
> > b/arch/powerpc/kernel/optprobes.c
> > index 024f7aad1952..445b3dad82dc 100644
> > --- a/arch/powerpc/kernel/optprobes.c
> > +++ b/arch/powerpc/kernel/optprobes.c
> > @@ -251,15 +251,17 @@ int arch_prepare_optimized_kprobe(struct
> > optimized_kprobe *op, struct kprobe *p)
> > goto error;
> > }
> >
> > - branch_op_callback = create_branch((unsigned int *)buff +
> > TMPL_CALL_HDLR_IDX,
> > - (unsigned long)op_callback_addr,
> > - BRANCH_SET_LINK);
> > + rc = create_branch(_op_callback,
> > +(unsigned int *)buff + TMPL_CALL_HDLR_IDX,
> > +(unsigned long)op_callback_addr,
> > +BRANCH_SET_LINK);
> >
> > - branch_emulate_step = create_branch((unsigned int *)buff +
> > TMPL_EMULATE_IDX,
> > - (unsigned long)emulate_step_addr,
> > - BRANCH_SET_LINK);
> > + rc |= create_branch(_emulate_step,
> > + (unsigned int *)buff + TMPL_EMULATE_IDX,
> > + (unsigned long)emulate_step_addr,
> > + BRANCH_SET_LINK);
> >
> > - if (!branch_op_callback || !branch_emulate_step)
> > + if (rc)
> > goto error;
> >
> > patch_instruction(buff + TMPL_CALL_HDLR_IDX, branch_op_callback);
> > @@ -305,6 +307,7 @@ int arch_check_optimized_kprobe(struct optimized_kprobe
> > *op)
> >
> > void arch_optimize_kprobes(struct list_head *oplist)
> > {
> > + unsigned int instr;
> > struct optimized_kprobe *op;
> > struct optimized_kprobe *tmp;
> >
> > @@ -315,9 +318,10 @@ void arch_optimize_kprobes(struct list_head *oplist)
> >*/
> > memcpy(op->optinsn.copied_insn, op->kp.addr,
> > RELATIVEJUMP_SIZE);
> > - patch_instruction(op->kp.addr,
> > - create_branch((unsigned int *)op->kp.addr,
> > - (unsigned long)op->optinsn.insn, 0));
> > + create_branch(,
> > +
Re: [PATCH v5 03/21] powerpc: Change calling convention for create_branch() et. al.
On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> create_branch(), create_cond_branch() and translate_branch() return the
> instruction that they create, or return 0 to signal an error. Seperate
s/seperate/separate
> these concerns in preparation for an instruction type that is not just
> an unsigned int. Fill the created instruction to a pointer passed as
> the first parameter to the function and use a non-zero return value to
> signify an error.
>
> Signed-off-by: Jordan Niethe
> ---
> v5: New to series
> ---
> arch/powerpc/include/asm/code-patching.h | 12 +-
> arch/powerpc/kernel/optprobes.c | 24 ++--
> arch/powerpc/kernel/setup_32.c | 2 +-
> arch/powerpc/kernel/trace/ftrace.c | 24 ++--
> arch/powerpc/lib/code-patching.c | 133 +--
> arch/powerpc/lib/feature-fixups.c| 5 +-
> 6 files changed, 117 insertions(+), 83 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h
> b/arch/powerpc/include/asm/code-patching.h
> index 898b54262881..351dda7215b6 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -22,10 +22,10 @@
> #define BRANCH_ABSOLUTE 0x2
>
> bool is_offset_in_branch_range(long offset);
> -unsigned int create_branch(const unsigned int *addr,
> -unsigned long target, int flags);
> -unsigned int create_cond_branch(const unsigned int *addr,
> - unsigned long target, int flags);
> +int create_branch(unsigned int *instr, const unsigned int *addr,
> + unsigned long target, int flags);
> +int create_cond_branch(unsigned int *instr, const unsigned int *addr,
> +unsigned long target, int flags);
> int patch_branch(unsigned int *addr, unsigned long target, int flags);
> int patch_instruction(unsigned int *addr, unsigned int instr);
> int raw_patch_instruction(unsigned int *addr, unsigned int instr);
> @@ -60,8 +60,8 @@ int instr_is_relative_branch(unsigned int instr);
> int instr_is_relative_link_branch(unsigned int instr);
> int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
> unsigned long branch_target(const unsigned int *instr);
> -unsigned int translate_branch(const unsigned int *dest,
> - const unsigned int *src);
> +int translate_branch(unsigned int *instr, const unsigned int *dest,
> + const unsigned int *src);
> extern bool is_conditional_branch(unsigned int instr);
> #ifdef CONFIG_PPC_BOOK3E_64
> void __patch_exception(int exc, unsigned long addr);
> diff --git a/arch/powerpc/kernel/optprobes.c
> b/arch/powerpc/kernel/optprobes.c
> index 024f7aad1952..445b3dad82dc 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -251,15 +251,17 @@ int arch_prepare_optimized_kprobe(struct
> optimized_kprobe *op, struct kprobe *p)
> goto error;
> }
>
> - branch_op_callback = create_branch((unsigned int *)buff +
> TMPL_CALL_HDLR_IDX,
> - (unsigned long)op_callback_addr,
> - BRANCH_SET_LINK);
> + rc = create_branch(_op_callback,
> +(unsigned int *)buff + TMPL_CALL_HDLR_IDX,
> +(unsigned long)op_callback_addr,
> +BRANCH_SET_LINK);
>
> - branch_emulate_step = create_branch((unsigned int *)buff +
> TMPL_EMULATE_IDX,
> - (unsigned long)emulate_step_addr,
> - BRANCH_SET_LINK);
> + rc |= create_branch(_emulate_step,
> + (unsigned int *)buff + TMPL_EMULATE_IDX,
> + (unsigned long)emulate_step_addr,
> + BRANCH_SET_LINK);
>
> - if (!branch_op_callback || !branch_emulate_step)
> + if (rc)
> goto error;
>
> patch_instruction(buff + TMPL_CALL_HDLR_IDX, branch_op_callback);
> @@ -305,6 +307,7 @@ int arch_check_optimized_kprobe(struct optimized_kprobe
> *op)
>
> void arch_optimize_kprobes(struct list_head *oplist)
> {
> + unsigned int instr;
> struct optimized_kprobe *op;
> struct optimized_kprobe *tmp;
>
> @@ -315,9 +318,10 @@ void arch_optimize_kprobes(struct list_head *oplist)
>*/
> memcpy(op->optinsn.copied_insn, op->kp.addr,
> RELATIVEJUMP_SIZE);
> - patch_instruction(op->kp.addr,
> - create_branch((unsigned int *)op->kp.addr,
> - (unsigned long)op->optinsn.insn, 0));
> + create_branch(,
> + (unsigned int *)op->kp.addr,
> + (unsigned long)op->optinsn.insn, 0);
> + patch_instruction(op->kp.addr, instr);
> list_del_init(>list);
> }
> }
> diff --git a/arch/powerpc/kernel/setup_32.c
Re: [PATCH v5 03/21] powerpc: Change calling convention for create_branch() et. al.
Hi Jordan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v5.6]
[also build test ERROR on next-20200406]
[cannot apply to powerpc/next kvm-ppc/kvm-ppc-next scottwood/next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Jordan-Niethe/Initial-Prefixed-Instruction-support/20200406-165215
base:7111951b8d4973bda27ff663f2cf18b663d15b48
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
arch/powerpc/kernel/setup_32.c: In function 'machine_init':
>> arch/powerpc/kernel/setup_32.c:91:21: error: passing argument 1 of
>> 'create_cond_branch' from incompatible pointer type
>> [-Werror=incompatible-pointer-types]
91 | create_cond_branch(, addr, branch_target(addr), 0x82);
| ^
| |
| long unsigned int *
In file included from arch/powerpc/kernel/setup_32.c:42:
arch/powerpc/include/asm/code-patching.h:27:38: note: expected 'unsigned int
*' but argument is of type 'long unsigned int *'
27 | int create_cond_branch(unsigned int *instr, const unsigned int *addr,
|~~^
cc1: all warnings being treated as errors
vim +/create_cond_branch +91 arch/powerpc/kernel/setup_32.c
67
68 /*
69 * This is run before start_kernel(), the kernel has been relocated
70 * and we are running with enough of the MMU enabled to have our
71 * proper kernel virtual addresses
72 *
73 * We do the initial parsing of the flat device-tree and prepares
74 * for the MMU to be fully initialized.
75 */
76 notrace void __init machine_init(u64 dt_ptr)
77 {
78 unsigned int *addr = (unsigned int
*)patch_site_addr(__memset_nocache);
79 unsigned long insn;
80
81 /* Configure static keys first, now that we're relocated. */
82 setup_feature_keys();
83
84 early_ioremap_setup();
85
86 /* Enable early debugging if any specified (see udbg.h) */
87 udbg_early_init();
88
89 patch_instruction_site(__memcpy_nocache, PPC_INST_NOP);
90
> 91 create_cond_branch(, addr, branch_target(addr), 0x82);
92 patch_instruction(addr, insn); /* replace b by bne cr0 */
93
94 /* Do some early initialization based on the flat device tree */
95 early_init_devtree(__va(dt_ptr));
96
97 early_init_mmu();
98
99 setup_kdump_trampoline();
100 }
101
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
