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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 03B00C4167B for ; Tue, 28 Nov 2023 08:52:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:References :In-Reply-To:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=d8crI5cDSIo53PjDS+pQ9TRNFYWKBFKlIiOAn/4hM90=; b=sdZdB1yvKD0bNM FXFQTchjuS3vqRQyN9IMmZ4TBiAZu4stU1dQRJCwAUNhhLh0zDI+JvoUjdxfSCW1ED/BG+VeN0A93 PAXCe55ODyPCGgsg+uS9Xr1iQS2PS49RMp5OUw+aplISEUUD0vpL6aWbH+xm5q5HnI8GkpZ9Dg7yH 9+wr0Nq44vhv1+A/OX3SxY/u0LAwBcN5Bf8PvwTDI/aqpNCq2t5wlx3u7GN7vtWa7XEbcfzxGwq+r Knhf7oxNF4461sSi9bnmRXL+Ytdw7HmohUkNW7rixPV1Dxu0UUvTgboI2R9GvhJGcR92zfWbPEAlq zux+RLk3jl1txpcinfcA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r7tpQ-004bDN-1Q; Tue, 28 Nov 2023 08:52:24 +0000 Received: from mail-lf1-x130.google.com ([2a00:1450:4864:20::130]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1r7tpM-004bCk-2J for linux-arm-kernel@lists.infradead.org; Tue, 28 Nov 2023 08:52:22 +0000 Received: by mail-lf1-x130.google.com with SMTP id 2adb3069b0e04-50bbb4de875so419237e87.0 for ; Tue, 28 Nov 2023 00:52:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701161538; x=1701766338; darn=lists.infradead.org; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:from:to:cc:subject:date:message-id:reply-to; bh=Qq9KZH3zwgyf5ps0zi7nf8nyXuIWJ/PJefkOKDRc2nE=; b=VfwxZnNiNMfOAPzjKIUfgXRaiZgEnwaJHqf8MKv3xbzRr4Vaq4yFJ7gSGa+cufAZzO HUbXr9sanwL1t+OWezPVAGei1QXp+cJ7gAEZnJ7NfL4ZwTH85xWgrJzHegotLfo+w+aa wdkh9/Dz7rlELJXefhCoC9GyF5QVomP8UorhERHng2X4o9apmmBgPnl5r+ERoTk2yt8m 1YfgvUbeNWdZUoElBeoqxu+BGbdZU7z1iVlPWtQ+hUKYDEDappHxiKd0Fz4xuZT/Q2Ka 7YPRTJKZk//rHhQTk3P8EEcVwqaHcpihb1vijyDSB4uFudOpLTc2faVpNQ56LqJLBFgY mtng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701161538; x=1701766338; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Qq9KZH3zwgyf5ps0zi7nf8nyXuIWJ/PJefkOKDRc2nE=; b=o8aUuCiE7RPQwdDnIsBeIcYPKr+bfiuYHXqpZmpoGpOAxJElUuUWEokAKN+WBv3ln/ 9OuA3lUa/x5Ewg1DXERGeBRF1CTx03XFGJxKbpPOvIWKTeSb8ebOrDXBQ4kvAysQIGld B8phDmEgMUpY8sM/GsKfgi7VHmOe+cGgBcD33/BV7xmgckfb68AbMvy+CEHbSYD5cuKz qgwh8PjOQwHqLTp2OY6Ddfx8mUhV5jpNmlR0No0evT8tLbqdaH2/FaYzqKgTzeCZvqDv ttoNdyJu60QQsg6fGyDRu17jdD85SxtHUMSYdftOC+8rX4G23LuECrfJFuYYNEHGho8D Y1GQ== X-Gm-Message-State: AOJu0YxWfl5+zij+xkVm6z9arhWy0LeCy4WIuRzzUFJcrQKiFaWUWSkA GNwfgWT7a5UA5FDi+pfSEAI= X-Google-Smtp-Source: AGHT+IEFzScz3FCviFlM1LOzYDR6v91zq0+uN5g5u19wyeKq2d7eBviTek5mluvENQ9gAT43eNS3Jw== X-Received: by 2002:a05:6512:3a90:b0:509:2b81:fc40 with SMTP id q16-20020a0565123a9000b005092b81fc40mr7633015lfu.9.1701161538131; Tue, 28 Nov 2023 00:52:18 -0800 (PST) Received: from localhost (54-240-197-231.amazon.com. [54.240.197.231]) by smtp.gmail.com with ESMTPSA id fa20-20020a05600c519400b0040b3e26872dsm11670608wmb.8.2023.11.28.00.52.17 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 28 Nov 2023 00:52:17 -0800 (PST) From: Puranjay Mohan To: Mark Rutland , linux-arm-kernel@lists.infradead.org Cc: broonie@kernel.org, catalin.marinas@arm.com, kaleshsingh@google.com, madvenka@linux.microsoft.com, mark.rutland@arm.com, will@kernel.org Subject: Re: [PATCH 1/2] arm64: stacktrace: factor out kernel unwind state In-Reply-To: <20231124110511.2795958-2-mark.rutland@arm.com> References: <20231124110511.2795958-1-mark.rutland@arm.com> <20231124110511.2795958-2-mark.rutland@arm.com> Date: Tue, 28 Nov 2023 08:52:14 +0000 Message-ID: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231128_005220_773929_F2CAD98D X-CRM114-Status: GOOD ( 32.36 ) 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 Mark Rutland writes: > On arm64 we share some unwinding code between the regular kernel > unwinder and the KVM hyp unwinder. Some of this common code only matters > to the regular unwinder, e.g. the `kr_cur` and `task` fields in the > common struct unwind_state. > > We're likely to add more state which only matters for regular kernel > unwinding (or only for hyp unwinding). In preparation for such changes, > this patch factors out the kernel-specific state into a new struct > kunwind_state, and updates the kernel unwind code accordingly. > > There should be no functional change as a result of this patch. > Reviewed-by: Puranjay Mohan Thanks, Puranjay > Signed-off-by: Mark Rutland > Cc: Catalin Marinas > Cc: Kalesh Singh > Cc: Madhavan T. Venkataraman > Cc: Mark Brown > Cc: Puranjay Mohan > Cc: Will Deacon > --- > arch/arm64/include/asm/stacktrace/common.h | 19 +--- > arch/arm64/include/asm/stacktrace/nvhe.h | 2 +- > arch/arm64/kernel/stacktrace.c | 113 +++++++++++++-------- > 3 files changed, 74 insertions(+), 60 deletions(-) > > diff --git a/arch/arm64/include/asm/stacktrace/common.h b/arch/arm64/include/asm/stacktrace/common.h > index 508f734de46ee..f63dc654e545f 100644 > --- a/arch/arm64/include/asm/stacktrace/common.h > +++ b/arch/arm64/include/asm/stacktrace/common.h > @@ -9,7 +9,6 @@ > #ifndef __ASM_STACKTRACE_COMMON_H > #define __ASM_STACKTRACE_COMMON_H > > -#include > #include > > struct stack_info { > @@ -23,12 +22,6 @@ struct stack_info { > * @fp: The fp value in the frame record (or the real fp) > * @pc: The lr value in the frame record (or the real lr) > * > - * @kr_cur: When KRETPROBES is selected, holds the kretprobe instance > - * associated with the most recently encountered replacement lr > - * value. > - * > - * @task: The task being unwound. > - * > * @stack: The stack currently being unwound. > * @stacks: An array of stacks which can be unwound. > * @nr_stacks: The number of stacks in @stacks. > @@ -36,10 +29,6 @@ struct stack_info { > struct unwind_state { > unsigned long fp; > unsigned long pc; > -#ifdef CONFIG_KRETPROBES > - struct llist_node *kr_cur; > -#endif > - struct task_struct *task; > > struct stack_info stack; > struct stack_info *stacks; > @@ -66,14 +55,8 @@ static inline bool stackinfo_on_stack(const struct stack_info *info, > return true; > } > > -static inline void unwind_init_common(struct unwind_state *state, > - struct task_struct *task) > +static inline void unwind_init_common(struct unwind_state *state) > { > - state->task = task; > -#ifdef CONFIG_KRETPROBES > - state->kr_cur = NULL; > -#endif > - > state->stack = stackinfo_get_unknown(); > } > > diff --git a/arch/arm64/include/asm/stacktrace/nvhe.h b/arch/arm64/include/asm/stacktrace/nvhe.h > index 25ab83a315a76..44759281d0d43 100644 > --- a/arch/arm64/include/asm/stacktrace/nvhe.h > +++ b/arch/arm64/include/asm/stacktrace/nvhe.h > @@ -31,7 +31,7 @@ static inline void kvm_nvhe_unwind_init(struct unwind_state *state, > unsigned long fp, > unsigned long pc) > { > - unwind_init_common(state, NULL); > + unwind_init_common(state); > > state->fp = fp; > state->pc = pc; > diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c > index 17f66a74c745c..aafc89192787a 100644 > --- a/arch/arm64/kernel/stacktrace.c > +++ b/arch/arm64/kernel/stacktrace.c > @@ -8,6 +8,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -18,6 +19,31 @@ > #include > #include > > +/* > + * Kernel unwind state > + * > + * @common: Common unwind state. > + * @task: The task being unwound. > + * @kr_cur: When KRETPROBES is selected, holds the kretprobe instance > + * associated with the most recently encountered replacement lr > + * value. > + */ > +struct kunwind_state { > + struct unwind_state common; > + struct task_struct *task; > +#ifdef CONFIG_KRETPROBES > + struct llist_node *kr_cur; > +#endif > +}; > + > +static __always_inline void > +kunwind_init(struct kunwind_state *state, > + struct task_struct *task) > +{ > + unwind_init_common(&state->common); > + state->task = task; > +} > + > /* > * Start an unwind from a pt_regs. > * > @@ -26,13 +52,13 @@ > * The regs must be on a stack currently owned by the calling task. > */ > static __always_inline void > -unwind_init_from_regs(struct unwind_state *state, > - struct pt_regs *regs) > +kunwind_init_from_regs(struct kunwind_state *state, > + struct pt_regs *regs) > { > - unwind_init_common(state, current); > + kunwind_init(state, current); > > - state->fp = regs->regs[29]; > - state->pc = regs->pc; > + state->common.fp = regs->regs[29]; > + state->common.pc = regs->pc; > } > > /* > @@ -44,12 +70,12 @@ unwind_init_from_regs(struct unwind_state *state, > * The function which invokes this must be noinline. > */ > static __always_inline void > -unwind_init_from_caller(struct unwind_state *state) > +kunwind_init_from_caller(struct kunwind_state *state) > { > - unwind_init_common(state, current); > + kunwind_init(state, current); > > - state->fp = (unsigned long)__builtin_frame_address(1); > - state->pc = (unsigned long)__builtin_return_address(0); > + state->common.fp = (unsigned long)__builtin_frame_address(1); > + state->common.pc = (unsigned long)__builtin_return_address(0); > } > > /* > @@ -63,35 +89,38 @@ unwind_init_from_caller(struct unwind_state *state) > * call this for the current task. > */ > static __always_inline void > -unwind_init_from_task(struct unwind_state *state, > - struct task_struct *task) > +kunwind_init_from_task(struct kunwind_state *state, > + struct task_struct *task) > { > - unwind_init_common(state, task); > + kunwind_init(state, task); > > - state->fp = thread_saved_fp(task); > - state->pc = thread_saved_pc(task); > + state->common.fp = thread_saved_fp(task); > + state->common.pc = thread_saved_pc(task); > } > > static __always_inline int > -unwind_recover_return_address(struct unwind_state *state) > +kunwind_recover_return_address(struct kunwind_state *state) > { > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > if (state->task->ret_stack && > - (state->pc == (unsigned long)return_to_handler)) { > + (state->common.pc == (unsigned long)return_to_handler)) { > unsigned long orig_pc; > - orig_pc = ftrace_graph_ret_addr(state->task, NULL, state->pc, > - (void *)state->fp); > - if (WARN_ON_ONCE(state->pc == orig_pc)) > + orig_pc = ftrace_graph_ret_addr(state->task, NULL, > + state->common.pc, > + (void *)state->common.fp); > + if (WARN_ON_ONCE(state->common.pc == orig_pc)) > return -EINVAL; > - state->pc = orig_pc; > + state->common.pc = orig_pc; > } > #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > > #ifdef CONFIG_KRETPROBES > - if (is_kretprobe_trampoline(state->pc)) { > - state->pc = kretprobe_find_ret_addr(state->task, > - (void *)state->fp, > - &state->kr_cur); > + if (is_kretprobe_trampoline(state->common.pc)) { > + unsigned long orig_pc; > + orig_pc = kretprobe_find_ret_addr(state->task, > + (void *)state->common.fp, > + &state->kr_cur); > + state->common.pc = orig_pc; > } > #endif /* CONFIG_KRETPROBES */ > > @@ -106,38 +135,38 @@ unwind_recover_return_address(struct unwind_state *state) > * and the location (but not the fp value) of B. > */ > static __always_inline int > -unwind_next(struct unwind_state *state) > +kunwind_next(struct kunwind_state *state) > { > struct task_struct *tsk = state->task; > - unsigned long fp = state->fp; > + unsigned long fp = state->common.fp; > int err; > > /* Final frame; nothing to unwind */ > if (fp == (unsigned long)task_pt_regs(tsk)->stackframe) > return -ENOENT; > > - err = unwind_next_frame_record(state); > + err = unwind_next_frame_record(&state->common); > if (err) > return err; > > - state->pc = ptrauth_strip_kernel_insn_pac(state->pc); > + state->common.pc = ptrauth_strip_kernel_insn_pac(state->common.pc); > > - return unwind_recover_return_address(state); > + return kunwind_recover_return_address(state); > } > > static __always_inline void > -unwind(struct unwind_state *state, stack_trace_consume_fn consume_entry, > - void *cookie) > +do_kunwind(struct kunwind_state *state, stack_trace_consume_fn consume_entry, > + void *cookie) > { > - if (unwind_recover_return_address(state)) > + if (kunwind_recover_return_address(state)) > return; > > while (1) { > int ret; > > - if (!consume_entry(cookie, state->pc)) > + if (!consume_entry(cookie, state->common.pc)) > break; > - ret = unwind_next(state); > + ret = kunwind_next(state); > if (ret < 0) > break; > } > @@ -190,22 +219,24 @@ noinline noinstr void arch_stack_walk(stack_trace_consume_fn consume_entry, > STACKINFO_EFI, > #endif > }; > - struct unwind_state state = { > - .stacks = stacks, > - .nr_stacks = ARRAY_SIZE(stacks), > + struct kunwind_state state = { > + .common = { > + .stacks = stacks, > + .nr_stacks = ARRAY_SIZE(stacks), > + }, > }; > > if (regs) { > if (task != current) > return; > - unwind_init_from_regs(&state, regs); > + kunwind_init_from_regs(&state, regs); > } else if (task == current) { > - unwind_init_from_caller(&state); > + kunwind_init_from_caller(&state); > } else { > - unwind_init_from_task(&state, task); > + kunwind_init_from_task(&state, task); > } > > - unwind(&state, consume_entry, cookie); > + do_kunwind(&state, consume_entry, cookie); > } > > static bool dump_backtrace_entry(void *arg, unsigned long where) > -- > 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel