Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 3 Mar 2021 14:55:33 +0000
From:      Andrew Turner <andrew@freebsd.org>
To:        Jessica Clarke <jrtc27@freebsd.org>
Cc:        "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org>
Subject:   Re: git: 28d945204ea1 - main - Handle functions that use a nop in the arm64 fbt
Message-ID:  <FCEC0A86-BF37-46B8-A7FB-34340ACE3460@freebsd.org>
In-Reply-To: <F46D1F1C-364B-427C-A1AD-059AD2CCF646@freebsd.org>
References:  <202103031426.123EQoM5082920@gitrepo.freebsd.org> <FF6A9C3C-82CB-45E1-83A5-166057192321@freebsd.org> <F46D1F1C-364B-427C-A1AD-059AD2CCF646@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

> On 3 Mar 2021, at 14:37, Jessica Clarke <jrtc27@freebsd.org> wrote:
>=20
> On 3 Mar 2021, at 14:29, Jessica Clarke <jrtc27@freebsd.org =
<mailto:jrtc27@freebsd.org>> wrote:
>> On 3 Mar 2021, at 14:26, Andrew Turner <andrew@FreeBSD.org> wrote:
>>>=20
>>> The branch main has been updated by andrew:
>>>=20
>>> URL: =
https://cgit.FreeBSD.org/src/commit/?id=3D28d945204ea1014d7de6906af8470ed8=
b3311335
>>>=20
>>> commit 28d945204ea1014d7de6906af8470ed8b3311335
>>> Author:     Andrew Turner <andrew@FreeBSD.org>
>>> AuthorDate: 2021-01-13 11:08:19 +0000
>>> Commit:     Andrew Turner <andrew@FreeBSD.org>
>>> CommitDate: 2021-03-03 14:18:03 +0000
>>>=20
>>>  Handle functions that use a nop in the arm64 fbt
>>>=20
>>>  To trace leaf asm functions we can insert a single nop instruction =
as
>>>  the first instruction in a function and trigger off this.
>>>=20
>>>  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(-)
>>>=20
>>> 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 @@
>>>=20
>>> #define	_C_LABEL(x)	x
>>>=20
>>> +#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)
>>=20
>> Doesn't this mean ENTRY incorrectly also has the nop?
>=20
> 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=E2=80=99t =
have the needed instructions to manage the stack frame as they don=E2=80=99=
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.

Andrew=



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?FCEC0A86-BF37-46B8-A7FB-34340ACE3460>