From patchwork Mon Apr 11 09:38:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sumit Garg X-Patchwork-Id: 12808780 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 38656C433EF for ; Mon, 11 Apr 2022 09:39:53 +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:References:In-Reply-To: Message-Id:Date: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=wWnhZ5BbO3PyxQIp2F8CqThXxhNq/N2YgX6lFWpJKkY=; b=BgnkEW9v1wDFtv LIDlCdh2ZsDt1LKR6i2j4gFQjxVXtBt7Qzp061xzV0s7V/tifscPkQ6PbbdUldpKQ6LhyLt5dlXW5 5tweAfgybyEUyOzl24nMB1KZy2r47dXnaFW+sGEUn1T8oIoUPnEpO1S0spLT6FWVUFUU1rCGg1Oj1 IYGJppC/XhN0J6VxEZWPObspKUJyLe6HGVLenuOVmg1wCMo+7XBqFTaKJpJNqOfTPt9NQf6nZoDf6 7+4umPv3czUx8aRuGSoDkLOvfBmjjT9FWw6BUZYe5fdFI7/ZEB+syKD4U3Sn67xIB02iRruVrEgT8 SwrMC+pZaBYtugKpPjnQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ndqVW-007yOp-HX; Mon, 11 Apr 2022 09:38:50 +0000 Received: from mail-pj1-x1029.google.com ([2607:f8b0:4864:20::1029]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ndqVO-007yNB-Jw for linux-arm-kernel@lists.infradead.org; Mon, 11 Apr 2022 09:38:44 +0000 Received: by mail-pj1-x1029.google.com with SMTP id ll10so6314529pjb.5 for ; Mon, 11 Apr 2022 02:38:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=D81f+bPtPMlxnVjBN4tpMYR04ACl9RkF3fcOQCuWauY=; b=fqEDwhsixdy86x/SmP+7NM6MCA1flbTTyQsOVkWS1VjiU69v7mE9DKHLS0tLtmO7e0 rC+7wwVgUzV5eieAo9ygj0D2eB+CukRKLUGgO2yky/FKwrMeP7C3p8MYaSZpaPpgIIwJ Bgp0KCfaY4Xjlzo62K2wMTnmLr6Dnwuf5uyC2J1giwC9P7lahH1Ff8cpnQamxg5LWEXr aIpJhLBhzB+kQKUUls/h9G2vwvl1Jbzlv4stwpq+uJwZsjsCNwKxq8T+hTjNtnIpgbif ePOO4Ws0HCaotvty3uwiS771v7xEOZdxd9R/jcQ8g3yU13FxKScHdL+acYMvkt3HO++1 S6yg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=D81f+bPtPMlxnVjBN4tpMYR04ACl9RkF3fcOQCuWauY=; b=phcWItcZZjQobtr198ZCBc5TBTLUPXEdqVE7KhG3kPKqPKNtjLEj51zEV+ueaOIl/k gDZbx6YqFPdr2VoQhw7i1HF88nnl4VYCdUPC5Vsco98jMHxWiZmr0zo3F3PZLA677x4v 3fO1dXUAbpbZ3P3K8f0TfhJlmsmk6cq+o9/xzETuZYA0rr2tPw8xjgT/D7GFGBqZfHYQ 5p25OIGNlKw9Z5Nq2g3GU7IQvpA2d8QOy0KvG557oBuwftgXrQQyNy4pPZeV+HIF0FoE UImUfCBvdxbCwqJ/pAOYXZV4bNkDldFG2tPKUnDc051Xd6z4sDiQPdDr2tx/ncbrLMKt Nelg== X-Gm-Message-State: AOAM531YIwK2sNwXICX+t3tU05Zn0RkvlJd/LoZo+qDBEOxpPzn77Sp3 3I1TBsm8Iykhkc3IsZZlM9Ayf/OuXlKkTlOe X-Google-Smtp-Source: ABdhPJwdcwxx2nrrbryx7J1x7Yjni6FEfyJrVoRlOd/dwN5lKnx3IK1TtEAf2IZEd4bbA/+KtyuSSg== X-Received: by 2002:a17:90a:558a:b0:1ca:a819:d2d1 with SMTP id c10-20020a17090a558a00b001caa819d2d1mr35891842pji.126.1649669921195; Mon, 11 Apr 2022 02:38:41 -0700 (PDT) Received: from localhost.localdomain ([223.177.215.72]) by smtp.gmail.com with ESMTPSA id d6-20020a056a00244600b004f701135460sm36461596pfj.146.2022.04.11.02.38.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Apr 2022 02:38:40 -0700 (PDT) From: Sumit Garg To: linux-arm-kernel@lists.infradead.org, dianders@chromium.org, will@kernel.org, liwei391@huawei.com Cc: catalin.marinas@arm.com, mark.rutland@arm.com, mhiramat@kernel.org, daniel.thompson@linaro.org, jason.wessel@windriver.com, linux-kernel@vger.kernel.org, Sumit Garg Subject: [PATCH 1/2] arm64: kgdb: Fix incorrect single stepping into the irq handler Date: Mon, 11 Apr 2022 15:08:18 +0530 Message-Id: <20220411093819.1012583-2-sumit.garg@linaro.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220411093819.1012583-1-sumit.garg@linaro.org> References: <20220411093819.1012583-1-sumit.garg@linaro.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220411_023842_719828_DCF7E4EA X-CRM114-Status: GOOD ( 16.65 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org PSTATE.I and PSTATE.D are very important for single step working. Without disabling interrupt on local CPU, there is a chance of interrupt occurrence in the period of exception return and start of kgdb/kdb single-step, that result in wrongly single stepping into the interrupt handler. And if D bit is set then, it results into undefined exception and when it's handler enables dbg then single step exception is generated, not as expected. Currently when we execute single step in kdb/kgdb, we may see it jumps to the irq_handler (where PSTATE.D is cleared) instead of the next instruction. And a resume after single stepping into interrupt handler sometimes leads to unbalanced locking: [ 300.328300] WARNING: bad unlock balance detected! [ 300.328608] 5.18.0-rc1-00016-g3e732ebf7316-dirty #6 Not tainted [ 300.329058] ------------------------------------- [ 300.329298] sh/173 is trying to release lock (dbg_slave_lock) at: [ 300.329718] [] kgdb_cpu_enter+0x7ac/0x820 [ 300.330029] but there are no more locks to release! [ 300.330265] [ 300.330265] other info that might help us debug this: [ 300.330668] 4 locks held by sh/173: [ 300.330891] #0: ffff4f5e454d8438 (sb_writers#3){.+.+}-{0:0}, at: vfs_write+0x98/0x204 [ 300.331735] #1: ffffd57c973bc2f0 (dbg_slave_lock){+.+.}-{2:2}, at: kgdb_cpu_enter+0x5b4/0x820 [ 300.332259] #2: ffffd57c973a9460 (rcu_read_lock){....}-{1:2}, at: kgdb_cpu_enter+0xe0/0x820 [ 300.332717] #3: ffffd57c973bc2a8 (dbg_master_lock){....}-{2:2}, at: kgdb_cpu_enter+0x1ec/0x820 Add the save and restore work for single-step while enabling and disabling single stepping to maintain the PSTATE.I and PSTATE.D carefully. Fixes: 44679a4f142b ("arm64: KGDB: Add step debugging support") Co-developed-by: Wei Li Signed-off-by: Wei Li Signed-off-by: Sumit Garg --- arch/arm64/kernel/kgdb.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c index 2aede780fb80..653ad0d19f2f 100644 --- a/arch/arm64/kernel/kgdb.c +++ b/arch/arm64/kernel/kgdb.c @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -171,6 +172,30 @@ static void kgdb_arch_update_addr(struct pt_regs *regs, compiled_break = 0; } +/* + * Interrupts need to be disabled before single-step mode is set, and not + * re-enabled until after single-step mode ends. Without disabling interrupt + * on local CPU, there is a chance of interrupt occurrence in the period of + * exception return and start of kgdb/kdb single-step, that result in wrongly + * single stepping into the interrupt handler. Also, resume from single + * stepping the interrupt handler is risky as it sometimes leads to unbalanced + * locking. + */ +static DEFINE_PER_CPU(unsigned long, kgdb_ss_flags); + +static void kgdb_save_local_irqflag(struct pt_regs *regs) +{ + __this_cpu_write(kgdb_ss_flags, (regs->pstate & DAIF_MASK)); + regs->pstate |= PSR_I_BIT; + regs->pstate &= ~PSR_D_BIT; +} + +static void kgdb_restore_local_irqflag(struct pt_regs *regs) +{ + regs->pstate &= ~DAIF_MASK; + regs->pstate |= __this_cpu_read(kgdb_ss_flags); +} + int kgdb_arch_handle_exception(int exception_vector, int signo, int err_code, char *remcom_in_buffer, char *remcom_out_buffer, @@ -201,8 +226,10 @@ int kgdb_arch_handle_exception(int exception_vector, int signo, /* * Received continue command, disable single step */ - if (kernel_active_single_step()) + if (kernel_active_single_step()) { + kgdb_restore_local_irqflag(linux_regs); kernel_disable_single_step(); + } err = 0; break; @@ -222,8 +249,10 @@ int kgdb_arch_handle_exception(int exception_vector, int signo, /* * Enable single step handling */ - if (!kernel_active_single_step()) + if (!kernel_active_single_step()) { + kgdb_save_local_irqflag(linux_regs); kernel_enable_single_step(linux_regs); + } err = 0; break; default: