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>
index | next in thread | previous in thread | raw e-mail
> On 3 Mar 2021, at 14:37, Jessica Clarke <jrtc27@freebsd.org> wrote: > > 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: >>> >>> The branch main has been updated by andrew: >>> >>> URL: https://cgit.FreeBSD.org/src/commit/?id=28d945204ea1014d7de6906af8470ed8b3311335 >>> >>> 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 >>> >>> 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. Andrewhelp
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?FCEC0A86-BF37-46B8-A7FB-34340ACE3460>