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

[Song Chen <chensong_2000@189.cn>]

在 2022/12/23 22:15, Masami Hiramatsu (Google) 写道:
> 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,
I tried not no add a new header file, but looks like i have to, to avoid 
triggering warnings and errors of kernel robot.

Thanks

Song

> 
>> 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
>>
> 
>