From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B4701C433DB for ; Tue, 23 Mar 2021 10:36:41 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 44E3A619BA for ; Tue, 23 Mar 2021 10:36:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 44E3A619BA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=28LrRuBjX8ptVZWmdBf9YiE7K3mle+KPhmVcUUKr1Es=; b=cl1UHRxL1RIxxkL8QbUKSU+iQ T8tuzce275nmt6TAiwrRo763Srid+lT0hk1kgq0F7oyPEhvbZtCrAi+mXo4kd9HHHfOQ2mvPNhnfG yMQ3Z4CFNBUqgQAUG+6GeSJOD6YGUc/7xy+AeIqGNC8NtWHMLp746iMOl7741+l64e+w73eW27L4b BHdJwy+SfEgfqCrpKdyKWer/0i7srGWp/AcJfKndren9FMnfPEKPXydULVtX0gCeevhbcYABn+y6/ VzncAb+Uf1RlixVvVutPbfUSUsR+xAY6+aHhpzQnSyQ/bXGq6gmEsG9+NtAi10UEOngk+fyQ0ZZrW 5zY7Rg1VQ==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lOeNP-00EdAP-1G; Tue, 23 Mar 2021 10:35:07 +0000 Received: from foss.arm.com ([217.140.110.172]) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lOeNJ-00Ed8j-PJ for linux-arm-kernel@lists.infradead.org; Tue, 23 Mar 2021 10:35:04 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0FB981042; Tue, 23 Mar 2021 03:34:59 -0700 (PDT) Received: from C02TD0UTHF1T.local (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D4F203F719; Tue, 23 Mar 2021 03:34:56 -0700 (PDT) Date: Tue, 23 Mar 2021 10:34:53 +0000 From: Mark Rutland To: madvenka@linux.microsoft.com Cc: broonie@kernel.org, jpoimboe@redhat.com, jthierry@redhat.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v2 2/8] arm64: Implement frame types Message-ID: <20210323103453.GB95840@C02TD0UTHF1T.local> References: <5997dfe8d261a3a543667b83c902883c1e4bd270> <20210315165800.5948-1-madvenka@linux.microsoft.com> <20210315165800.5948-3-madvenka@linux.microsoft.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210315165800.5948-3-madvenka@linux.microsoft.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210323_103502_253415_847916CB X-CRM114-Status: GOOD ( 33.95 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Mar 15, 2021 at 11:57:54AM -0500, madvenka@linux.microsoft.com wrote: > From: "Madhavan T. Venkataraman" > > Apart from the task pt_regs, pt_regs is also created on the stack for other > other cases: > > - EL1 exception. A pt_regs is created on the stack to save register > state. In addition, pt_regs->stackframe is set up for the > interrupted kernel function so that the function shows up in the > EL1 exception stack trace. > > - When a traced function calls the ftrace infrastructure at the > beginning of the function, ftrace creates a pt_regs on the stack > at that point to save register state. In addition, it sets up > pt_regs->stackframe for the traced function so that the traced > function shows up in the stack trace taken from anywhere in the > ftrace code after that point. When the ftrace code returns to the > traced function, the pt_regs is removed from the stack. > > To summarize, pt_regs->stackframe is used (or will be used) as a marker > frame in stack traces. To enable the unwinder to detect these frames, tag > each pt_regs->stackframe with a type. To record the type, use the unused2 > field in struct pt_regs and rename it to frame_type. The types are: > > TASK_FRAME > Terminating frame for a normal stack trace. > EL0_FRAME > Terminating frame for an EL0 exception. > EL1_FRAME > EL1 exception frame. > FTRACE_FRAME > FTRACE frame. > > These frame types will be used by the unwinder later to validate frames. I don't think that we need a marker in the pt_regs: * For kernel tasks and user tasks we just need the terminal frame record to be at a known location. We don't need the pt_regs to determine this. * For EL1<->EL1 exception boundaries, we already chain the frame records together, and we can identify the entry functions to see that there's an exception boundary. We don't need the pt_regs to determine this. * For ftrace using patchable-function-entry, we can identify the trampoline function. I'm also hoping to move away from pt_regs to an ftrace_regs here, and I'd like to avoid more strongly coupling this to pt_regs. Maybe I'm missing something you need for this last case? > > Signed-off-by: Madhavan T. Venkataraman > --- > arch/arm64/include/asm/ptrace.h | 15 +++++++++++++-- > arch/arm64/kernel/asm-offsets.c | 1 + > arch/arm64/kernel/entry.S | 4 ++++ > arch/arm64/kernel/head.S | 2 ++ > arch/arm64/kernel/process.c | 1 + > 5 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h > index e58bca832dff..a75211ce009a 100644 > --- a/arch/arm64/include/asm/ptrace.h > +++ b/arch/arm64/include/asm/ptrace.h > @@ -117,6 +117,17 @@ > */ > #define NO_SYSCALL (-1) > > +/* > + * pt_regs->stackframe is a marker frame that is used in different > + * situations. These are the different types of frames. Use patterns > + * for the frame types instead of (0, 1, 2, 3, ..) so that it is less > + * likely to find them on the stack. > + */ > +#define TASK_FRAME 0xDEADBEE0 /* Task stack termination frame */ > +#define EL0_FRAME 0xDEADBEE1 /* EL0 exception frame */ > +#define EL1_FRAME 0xDEADBEE2 /* EL1 exception frame */ > +#define FTRACE_FRAME 0xDEADBEE3 /* FTrace frame */ This sounds like we're using this as a heuristic, which I don't think we should do. I'd strongly prefr to avoid magic valuess here, and if we cannot be 100% certain of the stack contents, this is not reliable anyway. Thanks, Mark. > #ifndef __ASSEMBLY__ > #include > #include > @@ -187,11 +198,11 @@ struct pt_regs { > }; > u64 orig_x0; > #ifdef __AARCH64EB__ > - u32 unused2; > + u32 frame_type; > s32 syscallno; > #else > s32 syscallno; > - u32 unused2; > + u32 frame_type; > #endif > u64 sdei_ttbr1; > /* Only valid when ARM64_HAS_IRQ_PRIO_MASKING is enabled. */ > diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c > index a36e2fc330d4..43f97dbc7dfc 100644 > --- a/arch/arm64/kernel/asm-offsets.c > +++ b/arch/arm64/kernel/asm-offsets.c > @@ -75,6 +75,7 @@ int main(void) > DEFINE(S_SDEI_TTBR1, offsetof(struct pt_regs, sdei_ttbr1)); > DEFINE(S_PMR_SAVE, offsetof(struct pt_regs, pmr_save)); > DEFINE(S_STACKFRAME, offsetof(struct pt_regs, stackframe)); > + DEFINE(S_FRAME_TYPE, offsetof(struct pt_regs, frame_type)); > DEFINE(PT_REGS_SIZE, sizeof(struct pt_regs)); > BLANK(); > #ifdef CONFIG_COMPAT > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > index e2dc2e998934..ecc3507d9cdd 100644 > --- a/arch/arm64/kernel/entry.S > +++ b/arch/arm64/kernel/entry.S > @@ -269,8 +269,12 @@ alternative_else_nop_endif > */ > .if \el == 0 > stp xzr, xzr, [sp, #S_STACKFRAME] > + ldr w17, =EL0_FRAME > + str w17, [sp, #S_FRAME_TYPE] > .else > stp x29, x22, [sp, #S_STACKFRAME] > + ldr w17, =EL1_FRAME > + str w17, [sp, #S_FRAME_TYPE] > .endif > add x29, sp, #S_STACKFRAME > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S > index 2769b20934d4..d2ee78f8f97f 100644 > --- a/arch/arm64/kernel/head.S > +++ b/arch/arm64/kernel/head.S > @@ -410,6 +410,8 @@ SYM_FUNC_END(__create_page_tables) > */ > .macro setup_last_frame > sub sp, sp, #PT_REGS_SIZE > + ldr w17, =TASK_FRAME > + str w17, [sp, #S_FRAME_TYPE] > stp xzr, xzr, [sp, #S_STACKFRAME] > add x29, sp, #S_STACKFRAME > ldr x30, =ret_from_fork > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index 7ffa689e8b60..5c152fd60503 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -442,6 +442,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start, > * as the last frame for the new task. > */ > p->thread.cpu_context.fp = (unsigned long)childregs->stackframe; > + childregs->frame_type = TASK_FRAME; > > ptrace_hw_copy_thread(p); > > -- > 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel