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