Date: Wed, 3 Mar 2021 14:59:43 +0000 From: Jessica Clarke <jrtc27@freebsd.org> To: Andrew Turner <andrew@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: <8B945F5B-AC79-47E6-98D9-16762D3DBBF5@freebsd.org> In-Reply-To: <FCEC0A86-BF37-46B8-A7FB-34340ACE3460@freebsd.org> References: <202103031426.123EQoM5082920@gitrepo.freebsd.org> <FF6A9C3C-82CB-45E1-83A5-166057192321@freebsd.org> <F46D1F1C-364B-427C-A1AD-059AD2CCF646@freebsd.org> <FCEC0A86-BF37-46B8-A7FB-34340ACE3460@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 3 Mar 2021, at 14:55, Andrew Turner <andrew@FreeBSD.org> wrote: >=20 >> 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> 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? >=20 > 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. 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. Jess
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8B945F5B-AC79-47E6-98D9-16762D3DBBF5>