Message-ID: <f7e8243a-a4c8-44ce-ad03-7d232df461ed@linux.ibm.com>
Date: Mon, 4 Nov 2024 23:06:23 +0530
From: Hari Bathini <hbathini@...ux.ibm.com>
To: Segher Boessenkool <segher@...nel.crashing.org>
Cc: Shuah Khan <shuah@...nel.org>, linux-kselftest@...r.kernel.org,
        Steven Rostedt <rostedt@...dmis.org>,
        Masami Hiramatsu
 <mhiramat@...nel.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Madhavan Srinivasan <maddy@...ux.ibm.com>,
        "Naveen N. Rao"
 <naveen@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        linux-trace-kernel@...r.kernel.org,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH] selftests/ftrace: update kprobe syntax error test for
 ppc64le

Hi Segher,

On 04/11/24 4:06 pm, Segher Boessenkool wrote:
> Hi!
> 
> On Mon, Nov 04, 2024 at 03:40:26PM +0530, Hari Bathini wrote:
>> On 04/11/24 3:14 pm, Segher Boessenkool wrote:
>>> On Mon, Nov 04, 2024 at 02:51:57PM +0530, Hari Bathini wrote:
>>>> On 02/11/24 2:29 am, Segher Boessenkool wrote:
>>>>> On Sat, Nov 02, 2024 at 12:49:25AM +0530, Hari Bathini wrote:
>>>>>> For ppc64le, depending on the kernel configuration used, offset 16
>>>>> >from function start address can also be considered function entry.
>>>>>> Update the test case to accommodate such configurations.
>>>>>
>>>>> (This is true for all ELfv2, not just LE.  For the kernel that is about
>>>>> the same).
>>>>>
>>>>> The LEP and GEP can differ by zero, one, two, four, eight, or sixteen
>>>>> insns (where an insn is four bytes).  Four insns is common, yes, but
>>>>> maybe you can support all?  See the function symbol's st_other field
>>>>> to see what the offset is:
>>>>> 0, 1: zero insns, zero bytes
>>>>> N = 2..6: 1 << (N-2) insns, i.e. 1<<N bytes
>>>>> 7: reserved
>>>>>
>>>>> (This is the top 3 bits of st_other, the other bits have other meanings).
>>>>>
>>>>> Four insns is common, yes, but by no means the only possibility.
>>>>
>>>> Hi Segher,
>>>>
>>>> Querying for function arguments is supported on kprobes only at function
>>>> entry. This is a negative test case where the offset is intentionally
>>>> set beyond function entry while querying for function arguments.
>>>> I guess, simply setting the offset to 20 (vfs_read is anyway
>>>> going to be beyond 5 instructions) instead of 8 for powerpc would
>>>> make all platforms and ABI variants happy?
>>>
>>> I have no idea.  What is this "offset" anyway?
>>
>> offset (in bytes) from function start address..
> 
> But what is there?
> 
>>> This is just the ELFv2 ABI.  No platform can make up its own thing at
>>> all (well, none decided to be gratuitously incompatible, so far).  And
>>> there are no "ABI variants"!
>>
>> The test case applies for ABIv1 & ABIv2. All ppc32 & ppc64 platforms..
> 
> Hrm.  So you allow essentially random entry points on other ABIs to
> work?
> 
>>> You're just making assumptions here that are based on nothing else but
>>> observations of what is done most of the time.  That might work for a
>>> while -- maybe a long while even! -- but it can easily break down.
>>
>> Hmmm.. I understand that you want the test case to read st_other field
>> but would you rather suggest an offset of 64?
> 
> I have no idea what "offset" means here.
> 
>> Is a GEP of 8/16 instructions going to be true anytime soon or is it
>> true already for some cases? The reason I ask that is some kprobe/ftrace
>> code in the kernel might need a bit of re-look if that is the case.
> 
> An entry point has no instructions at all.  Oh, you mean the code at
> the GEP.
> 
> The LEP can already be all the allowed distances after the GEP.  And
> the .localentry GAS directive already supports all those distances
> always.  Not a lot of code written in assembler does use that, and
> certainly GCC does not use a lot of the freedom it has here, but it
> could (and so could assembler programmers).  Typically people will want
> to make the code here as short as possible, and there are restrictions
> on what is *allowed* to be done here anyway (ld, the link editor, can
> change this code after all!), so it is not too likely you will ever see
> big code at the GEP often, but times change, etc.

Seems like a bit of misunderstanding there. Function entry here intends
to mean the actual start of function code (function prologue) - after
GEP and function profiling sequence (mflr r0; bl mcount).

Function arguments can be accessed with kprobe only while setting a
probe at an address the kernel treats as function start address.
Note that the test case pass criteria here is setting probe to fail by
providing an address (sym+offset) beyond the function start address.

And in this specific test case (with "vfs_read+8", where vfs_read is
the symbol and '8' is the offset), the test case was failing on powerpc
because setting the probe at 'sym+8' was succeeding, as anywhere between
'sym' to 'sym+16' is treated as function start address on powerpc:

  
https://github.com/torvalds/linux/blob/master/arch/powerpc/kernel/kprobes.c#L108


So, the fix here essentially is to provide an address that is at least
an insn or two beyond function start address. As GEP is 8 bytes and
function profile sequence is 8 bytes, sym+20 is beyond function start
address on ppc64le. In fact, sym+20 should work for other platforms
too as sym+20 not treated as function start address on any platform
on powerpc as of today, and that is all the test case cares about...

Thanks
Hari

Powered by blists - more mailing lists