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 CB9F7C3ABCC for ; Mon, 12 May 2025 18:08:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-ID:Date :Subject:To:From:Reply-To:Content-Type:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=QIs/o0ZkmpcRztFm0YJac37BmBMTukI27Mx5/tNLch4=; b=uQCWkn/U80fbTn BW+nkG1gYfRSR2o6R09aZS8t+fW+pVxd97c7GlKcUiCyRU4S9I2kdRfRtISTyl8zDC5NdpOaLTUs/ WaniwDYIf37rmxxDBV8fvpvwg0ezgLvvIDDpHTN+x15ysctZPHSB+zJ8rwi3AUwfdxsnWhpWv6te+ tbAVv991RVdnrpCa9hX8HKo2nBDoDkKl7fGd+m++yMTWX/63mZgv4ZN+oMc0Qna//MUtdzJHjP1qe +85zW1QI9TDxra93ayXTZCSujMyPbKtZl2tyC6zDcEikHmpMtb/WmvoRgDw78gS4HwDT9SHCqj/Xx naF2pDq7sm7JkZItGxHQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uEXZv-0000000AIBZ-1s10; Mon, 12 May 2025 18:08:39 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uEXC4-0000000AFC8-2xnD for linux-arm-kernel@lists.infradead.org; Mon, 12 May 2025 17:44:02 +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 5A58D28AC; Mon, 12 May 2025 10:43:49 -0700 (PDT) Received: from e137867.cambridge.arm.com (e137867.arm.com [10.1.32.174]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 0AF5A3F673; Mon, 12 May 2025 10:43:58 -0700 (PDT) From: Ada Couprie Diaz To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 07/11] arm64: debug: split single stepping exception entry Date: Mon, 12 May 2025 18:43:22 +0100 Message-ID: <20250512174326.133905-8-ada.coupriediaz@arm.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250512174326.133905-1-ada.coupriediaz@arm.com> References: <20250512174326.133905-1-ada.coupriediaz@arm.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250512_104400_836756_625CD417 X-CRM114-Status: GOOD ( 22.91 ) 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: , Cc: Mark Rutland , "Luis Claudio R. Goncalves" , Catalin Marinas , Sebastian Andrzej Siewior , Will Deacon Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Currently all debug exceptions share common entry code and are routed to `do_debug_exception()`, which calls dynamically-registered handlers for each specific debug exception. This is unfortunate as different debug exceptions have different entry handling requirements, and it would be better to handle these distinct requirements earlier. The single stepping exception has the most constraints : it can be exploited to train branch predictors and it needs special handling at EL1 for the Cortex-A76 erratum #1463225. We need to conserve all those mitigations. Move the call to `arm64_apply_bp_hardening()` to `entry-common.c` as it is needed for exceptions coming from EL0 only. However, it does not write an address at FAR_EL1, as only hardware watchpoints do so. The single-step handler does its own signaling if it needs to and only returns 0, so we can call it directly from `entry-common.c`. Split the single stepping exception entry, adjust the function signature, keep the security mitigation and erratum handling. When taking a soft-step exception from EL0, most of the single stepping handling is safely preemptible : the only possible handler is `uprobe_singlestep_handler()`. It only operates on task-local data and properly checks its validity, then raises a Thread Information Flag, processed before returning to userspace in `do_notify_resume()`, which is already preemptible. However, the soft-step handler first calls `reinstall_suspended_bps()` to check if there is any hardware breakpoint or watchpoint pending or already stepped through. This cannot be preempted as it manipulates the hardware breakpoint and watchpoint registers. Move the call to `reinstall_suspended_bps()` to `entry-common.c` and adjust the relevant comments. We can now safely unmask interrupts before handling the step itself, fixing a PREEMPT_RT issue where the handler could call a sleeping function with preemption disabled. Signed-off-by: Ada Couprie Diaz Closes: https://lore.kernel.org/linux-arm-kernel/Z6YW_Kx4S2tmj2BP@uudg.org/ --- arch/arm64/include/asm/exception.h | 1 + arch/arm64/kernel/debug-monitors.c | 19 +++---------- arch/arm64/kernel/entry-common.c | 43 ++++++++++++++++++++++++++++++ arch/arm64/kernel/hw_breakpoint.c | 6 ++--- 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h index c593fe639697..cbcd832bf58e 100644 --- a/arch/arm64/include/asm/exception.h +++ b/arch/arm64/include/asm/exception.h @@ -62,6 +62,7 @@ void do_el1_gcs(struct pt_regs *regs, unsigned long esr); void do_debug_exception(unsigned long addr_if_watchpoint, unsigned long esr, struct pt_regs *regs); void do_breakpoint(unsigned long esr, struct pt_regs *regs); +void do_softstep(unsigned long esr, struct pt_regs *regs); void do_fpsimd_acc(unsigned long esr, struct pt_regs *regs); void do_sve_acc(unsigned long esr, struct pt_regs *regs); void do_sme_acc(unsigned long esr, struct pt_regs *regs); diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c index 55bca019ef5c..d3cb8f5da51b 100644 --- a/arch/arm64/kernel/debug-monitors.c +++ b/arch/arm64/kernel/debug-monitors.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -197,18 +198,10 @@ static void send_user_sigtrap(int si_code) "User debug trap"); } -static int single_step_handler(unsigned long unused, unsigned long esr, - struct pt_regs *regs) +void do_softstep(unsigned long esr, struct pt_regs *regs) { - /* - * If we are stepping a pending breakpoint, call the hw_breakpoint - * handler first. - */ - if (!reinstall_suspended_bps(regs)) - return 0; - if (call_step_hook(regs, esr) == DBG_HOOK_HANDLED) - return 0; + return; if (user_mode(regs)) { send_user_sigtrap(TRAP_TRACE); @@ -228,10 +221,8 @@ static int single_step_handler(unsigned long unused, unsigned long esr, */ set_regs_spsr_ss(regs); } - - return 0; } -NOKPROBE_SYMBOL(single_step_handler); +NOKPROBE_SYMBOL(do_softstep); static int call_break_hook(struct pt_regs *regs, unsigned long esr) { @@ -341,8 +332,6 @@ NOKPROBE_SYMBOL(aarch32_break_handler); void __init debug_traps_init(void) { - hook_debug_fault_code(DBG_ESR_EVT_HWSS, single_step_handler, SIGTRAP, - TRAP_TRACE, "single-step handler"); hook_debug_fault_code(DBG_ESR_EVT_BRK, brk_handler, SIGTRAP, TRAP_BRKPT, "BRK handler"); } diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c index 6ff52fc94da7..8814ad24e707 100644 --- a/arch/arm64/kernel/entry-common.c +++ b/arch/arm64/kernel/entry-common.c @@ -512,6 +512,24 @@ static void noinstr el1_breakpt(struct pt_regs *regs, unsigned long esr) arm64_exit_el1_dbg(regs); } +static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr) +{ + arm64_enter_el1_dbg(regs); + if (!cortex_a76_erratum_1463225_debug_handler(regs)) { + debug_exception_enter(regs); + /* + * After handling a breakpoint, we suspend the breakpoint + * and use single-step to move to the next instruction. + * If we have a suspended breakpoint there's nothing more to do: + * complete the single-step. + */ + if (reinstall_suspended_bps(regs)) + do_softstep(esr, regs); + debug_exception_exit(regs); + } + arm64_exit_el1_dbg(regs); +} + static void noinstr el1_dbg(struct pt_regs *regs, unsigned long esr) { unsigned long far = read_sysreg(far_el1); @@ -564,6 +582,8 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs) el1_breakpt(regs, esr); break; case ESR_ELx_EC_SOFTSTP_CUR: + el1_softstp(regs, esr); + break; case ESR_ELx_EC_WATCHPT_CUR: case ESR_ELx_EC_BRK64: el1_dbg(regs, esr); @@ -770,6 +790,25 @@ static void noinstr el0_breakpt(struct pt_regs *regs, unsigned long esr) exit_to_user_mode(regs); } +static void noinstr el0_softstp(struct pt_regs *regs, unsigned long esr) +{ + if (!is_ttbr0_addr(regs->pc)) + arm64_apply_bp_hardening(); + + enter_from_user_mode(regs); + /* + * After handling a breakpoint, we suspend the breakpoint + * and use single-step to move to the next instruction. + * If we have a suspended breakpoint there's nothing more to do: + * complete the single-step. + */ + if (reinstall_suspended_bps(regs)) { + local_daif_restore(DAIF_PROCCTX); + do_softstep(esr, regs); + } + exit_to_user_mode(regs); +} + static void noinstr el0_dbg(struct pt_regs *regs, unsigned long esr) { /* Only watchpoints write FAR_EL1, otherwise its UNKNOWN */ @@ -851,6 +890,8 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs) el0_breakpt(regs, esr); break; case ESR_ELx_EC_SOFTSTP_LOW: + el0_softstp(regs, esr); + break; case ESR_ELx_EC_WATCHPT_LOW: case ESR_ELx_EC_BRK64: el0_dbg(regs, esr); @@ -973,6 +1014,8 @@ asmlinkage void noinstr el0t_32_sync_handler(struct pt_regs *regs) el0_breakpt(regs, esr); break; case ESR_ELx_EC_SOFTSTP_LOW: + el0_softstp(regs, esr); + break; case ESR_ELx_EC_WATCHPT_LOW: case ESR_ELx_EC_BKPT32: el0_dbg(regs, esr); diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c index b7253ddac230..1ab37d6561cb 100644 --- a/arch/arm64/kernel/hw_breakpoint.c +++ b/arch/arm64/kernel/hw_breakpoint.c @@ -866,9 +866,9 @@ int reinstall_suspended_bps(struct pt_regs *regs) kernel_step = this_cpu_ptr(&stepping_kernel_bp); /* - * Called from single-step exception handler. - * Return 0 if execution can resume, 1 if a SIGTRAP should be - * reported. + * Called from single-step exception entry. + * Return 0 if execution can resume, 1 if the single step + * should be handled. */ if (user_mode(regs)) { if (debug_info->bps_disabled) { -- 2.43.0