Re: [PATCH v3 4/4] kernel/trace: remove calling regs_* when compiling HEXAGON

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Mon,  5 Dec 2022 16:30:17 +0800
Song Chen <chensong_2000@189.cn> wrote:

> kernel test robot reports below errors:
> 
> In file included from kernel/trace/trace_events_synth.c:22:
> >> kernel/trace/trace_probe_kernel.h:203:9: error: call to
> undeclared function 'regs_get_register'; ISO C99 and later
> do not support implicit function declarations
> [-Wimplicit-function-declaration]
>                    val = regs_get_register(regs, code->param);
> 
> HEXAGON doesn't define and implement those reg_* functions
> underneath arch/hexagon as well as other archs. To remove
> those errors, i have to include those function calls in
> "CONFIG_HEXAGON"
> 
> It looks ugly, but i don't know any other way to fix it,
> this patch can be reverted after reg_* have been in place
> in arch/hexagon.
> 

Sorry, NACK. This is too add-hoc patch and this is introduced
by your patch. Do not introduce an issue and fix it later in
the same series.
Please fix it in your first patch. Maybe you should make another
header file for those APIs.

Thank you, 

> Signed-off-by: Song Chen <chensong_2000@189.cn>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
>  kernel/trace/trace_probe_kernel.h | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_probe_kernel.h b/kernel/trace/trace_probe_kernel.h
> index 8c42abe0dacf..7e958b7f07e5 100644
> --- a/kernel/trace/trace_probe_kernel.h
> +++ b/kernel/trace/trace_probe_kernel.h
> @@ -130,8 +130,7 @@ probe_mem_read(void *dest, void *src, size_t size)
>  	return copy_from_kernel_nofault(dest, src, size);
>  }
>  
> -static nokprobe_inline unsigned long
> -get_event_field(struct fetch_insn *code, void *rec)
> +static unsigned long get_event_field(struct fetch_insn *code, void *rec)
>  {
>  	struct ftrace_event_field *field = code->data;
>  	unsigned long val;
> @@ -194,23 +193,41 @@ static int
>  process_fetch_insn(struct fetch_insn *code, void *rec, void *dest,
>  		   void *base)
>  {
> +#ifndef CONFIG_HEXAGON
>  	struct pt_regs *regs = rec;
> +#endif
>  	unsigned long val;
>  
>  retry:
>  	/* 1st stage: get value from context */
>  	switch (code->op) {
>  	case FETCH_OP_REG:
> +#ifdef CONFIG_HEXAGON
> +		val = 0;
> +#else
>  		val = regs_get_register(regs, code->param);
> +#endif
>  		break;
>  	case FETCH_OP_STACK:
> +#ifdef CONFIG_HEXAGON
> +		val = 0;
> +#else
>  		val = regs_get_kernel_stack_nth(regs, code->param);
> +#endif
>  		break;
>  	case FETCH_OP_STACKP:
> +#ifdef CONFIG_HEXAGON
> +		val = 0;
> +#else
>  		val = kernel_stack_pointer(regs);
> +#endif
>  		break;
>  	case FETCH_OP_RETVAL:
> +#ifdef CONFIG_HEXAGON
> +		val = 0;
> +#else
>  		val = regs_return_value(regs);
> +#endif
>  		break;
>  	case FETCH_OP_IMM:
>  		val = code->immediate;
> -- 
> 2.25.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>