git: 28d945204ea1 - main - Handle functions that use a nop in the arm64 fbt

[Andrew Turner]

> On 3 Mar 2021, at 14:59, Jessica Clarke <jrtc27 at freebsd.org> wrote:
> 
> On 3 Mar 2021, at 14:55, Andrew Turner <andrew at FreeBSD.org <mailto:andrew at FreeBSD.org>> wrote:
>> 
>>> On 3 Mar 2021, at 14:37, Jessica Clarke <jrtc27 at freebsd.org> wrote:
>>> 
>>> On 3 Mar 2021, at 14:29, Jessica Clarke <jrtc27 at freebsd.org> wrote:
>>>> On 3 Mar 2021, at 14:26, Andrew Turner <andrew at FreeBSD.org> wrote:
>>>>> 
>>>>> The branch main has been updated by andrew:
>>>>> 
>>>>> URL: https://cgit.FreeBSD.org/src/commit/?id=28d945204ea1014d7de6906af8470ed8b3311335
>>>>> 
>>>>> commit 28d945204ea1014d7de6906af8470ed8b3311335
>>>>> Author:     Andrew Turner <andrew at FreeBSD.org>
>>>>> AuthorDate: 2021-01-13 11:08:19 +0000
>>>>> Commit:     Andrew Turner <andrew at FreeBSD.org>
>>>>> CommitDate: 2021-03-03 14:18:03 +0000
>>>>> 
>>>>> Handle functions that use a nop in the arm64 fbt
>>>>> 
>>>>> To trace leaf asm functions we can insert a single nop instruction as
>>>>> the first instruction in a function and trigger off this.
>>>>> 
>>>>> Reviewed by:    gnn
>>>>> Sponsored by:   Innovate UK
>>>>> Differential Revision:  https://reviews.freebsd.org/D28132
>>>>> ---
>>>>> sys/arm64/include/asm.h                            |  8 +++-
>>>>> .../contrib/opensolaris/uts/common/sys/dtrace.h    |  2 +
>>>>> sys/cddl/dev/dtrace/aarch64/dtrace_subr.c          |  5 +++
>>>>> sys/cddl/dev/fbt/aarch64/fbt_isa.c                 | 51 ++++++++++++++--------
>>>>> 4 files changed, 46 insertions(+), 20 deletions(-)
>>>>> 
>>>>> diff --git a/sys/arm64/include/asm.h b/sys/arm64/include/asm.h
>>>>> index 05e618500e59..32b79d256e80 100644
>>>>> --- a/sys/arm64/include/asm.h
>>>>> +++ b/sys/arm64/include/asm.h
>>>>> @@ -38,9 +38,15 @@
>>>>> 
>>>>> #define	_C_LABEL(x)	x
>>>>> 
>>>>> +#ifdef KDTRACE_HOOKS
>>>>> +#define	DTRACE_NOP	nop
>>>>> +#else
>>>>> +#define	DTRACE_NOP
>>>>> +#endif
>>>>> +
>>>>> #define	LENTRY(sym)						\
>>>>> 	.text; .align 2; .type sym,#function; sym:		\
>>>>> -	.cfi_startproc
>>>>> +	.cfi_startproc; DTRACE_NOP
>>>>> #define	ENTRY(sym)						\
>>>>> 	.globl sym; LENTRY(sym)
>>>> 
>>>> Doesn't this mean ENTRY incorrectly also has the nop?
>>> 
>>> Hm, right, the L in LENTRY means local not leaf. Isn't this a problem
>>> though? (L)ENTRY are perfectly legal to use for non-leaf assembly
>>> functions today. Shouldn't there be separate ones specifically for leaf
>>> functions if you want to treat them differently?
>> 
>> Other than early boot handling, pmap_switch, and the exception handlers I think we only have a few non-leaf asm functions on arm64. The only ones I can think of use tail recursion, e.g. memmove -> memcpy when possible. Other than exception handlers these functions don’t have the needed instructions to manage the stack frame as they don’t use any stack space. I decided it was easier to add the nop instruction to the start of function than try to create an unneeded stack frame.
> 
> I don't contest that. My problem is that there is now a hidden
> requirement that (L)ENTRY only be used for leaf functions lest you get
> broken FBT for them. That is a surprising restriction, which to me
> should be indicated by having a different macro name from the generic
> (L)ENTRY shared across most (all?) ports. Despite its flaws, MIPS does
> have special LEAF macros that are distinct from the others.

Why would you get broken FBT? All it cares about is finding an instruction it can emulate and replace it with a specific breakpoint. In a non-leaf asm function we will place a nop as the first instruction followed by the standard stack frame manipulation instructions. In this case the nop is unneeded, but should add minimal overhead if such a function is added.

I only mentioned leaf functions in the commit message as an example of something that we may not have previously been able to trace due to a lack of stack usage.

Andrew