Date: Wed, 3 Mar 2021 15:14:54 +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: <BCF4BFF1-C9CC-4E4E-94D6-3DC24DAD3932@freebsd.org> In-Reply-To: <9D7C6FED-2B65-4219-9B1E-4BAF5AC5CEC8@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> <8B945F5B-AC79-47E6-98D9-16762D3DBBF5@freebsd.org> <9D7C6FED-2B65-4219-9B1E-4BAF5AC5CEC8@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> On 3 Mar 2021, at 15:09, Andrew Turner <andrew@freebsd.org> wrote: >=20 >>=20 >> On 3 Mar 2021, at 14:59, Jessica Clarke <jrtc27@freebsd.org> wrote: >>=20 >> 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. >>=20 >> 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. >=20 > 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. >=20 > 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. Oh I see, I didn't read the commit properly, I assumed it was to watermark leaf functions for better stack traces. My bad. Jess
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BCF4BFF1-C9CC-4E4E-94D6-3DC24DAD3932>