From owner-dev-commits-src-all@freebsd.org Wed Mar 3 15:14:58 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 7DE6A56DB01 for ; Wed, 3 Mar 2021 15:14:58 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DrHc22zGdz3tjm for ; Wed, 3 Mar 2021 15:14:58 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: by mail-wm1-f41.google.com with SMTP id n4so6664951wmq.3 for ; Wed, 03 Mar 2021 07:14:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=KH7n4N8muXRs0tUlMFGUIz8Sm7UFW84+7FRVgfV/rd4=; b=G3DZ6SOsbO4k4UjInqmAsTK7NTa1HTTvoYd272+1chK474P734fNMjDjNvP35WOywm IsQCduhMafVF1EQdDPwt2vxXzUdhlggfh1zniN6kFhgdgNv+CNv6M7AyMR64zLJJj0er P2g2irrpoG/X/rKIWEWkBRKMolm+WjbbGquWIF9SSPm3InHWZI/aZnQa4Sh4/ytrFFkd 47Rf/JWKFtE0LCDt2Qx3As4IObcde/eKzpimxeeDMpoi2BJAz+4VU7OYv4bZfxSNHwQG ejdlD4J845rXrjFsAF7AQkUAaF+RcTiRo/l7wg639u4sufVZiBL7XScwtOcvH19LicPJ 8qXw== X-Gm-Message-State: AOAM530sQGMi1ZaVF2DvlwumxuwGyf5/38ZuniUhGs9AOHSvMZd2qnuB va9qScdkbrWZW7vjAG2nEjugsA== X-Google-Smtp-Source: ABdhPJxFXLyAiBPWIAUEY3AFgasjt7a09GCtbX4hD466mnrk/m9RlOvWpx68sDMEycb+exgOcQoQdg== X-Received: by 2002:a1c:a543:: with SMTP id o64mr9564706wme.107.1614784495597; Wed, 03 Mar 2021 07:14:55 -0800 (PST) Received: from [192.168.149.251] (trinity-students-nat.trin.cam.ac.uk. [131.111.193.104]) by smtp.gmail.com with ESMTPSA id b186sm4707048wmc.44.2021.03.03.07.14.55 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Mar 2021 07:14:55 -0800 (PST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.120.23.2.4\)) Subject: Re: git: 28d945204ea1 - main - Handle functions that use a nop in the arm64 fbt From: Jessica Clarke In-Reply-To: <9D7C6FED-2B65-4219-9B1E-4BAF5AC5CEC8@freebsd.org> Date: Wed, 3 Mar 2021 15:14:54 +0000 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: References: <202103031426.123EQoM5082920@gitrepo.freebsd.org> <8B945F5B-AC79-47E6-98D9-16762D3DBBF5@freebsd.org> <9D7C6FED-2B65-4219-9B1E-4BAF5AC5CEC8@freebsd.org> To: Andrew Turner X-Mailer: Apple Mail (2.3608.120.23.2.4) X-Rspamd-Queue-Id: 4DrHc22zGdz3tjm X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Mar 2021 15:14:58 -0000 > On 3 Mar 2021, at 15:09, Andrew Turner wrote: >=20 >>=20 >> On 3 Mar 2021, at 14:59, Jessica Clarke wrote: >>=20 >> On 3 Mar 2021, at 14:55, Andrew Turner wrote: >>>=20 >>>> On 3 Mar 2021, at 14:37, Jessica Clarke wrote: >>>>=20 >>>> On 3 Mar 2021, at 14:29, Jessica Clarke wrote: >>>>> On 3 Mar 2021, at 14:26, Andrew Turner 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 >>>>>> AuthorDate: 2021-01-13 11:08:19 +0000 >>>>>> Commit: Andrew Turner >>>>>> 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