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 44CEBC77B61 for ; Tue, 25 Apr 2023 13:41:01 +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: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=dTf2soNmsrr6FXc2ejLI1SKI+7qlxw5q+zHdy+y0Lls=; b=NQEMPrabI9b3Ft 95JODQ4QWS7FdJU0XsLu5M8VhKvjaxdDmMphnl+Ea22OyGEICIaS/mo34c1fWto+rzEIt6Q9WWAAs MB4i2sdmH8Ay/DUvbBVd9XfNLVGRKx9jU2wPbWqe6UUyp8oPp9peGF2fhf5JDjQ9IgzX2x74zAh7V badNQzQloGitMXoQZFKs22+/yUDGSpIoE5k6Hzx9uMd0+O7de0sgQD4EjBfrHqS0C6j776vs3caIM ZGbovRZ1gX9fsKH9mji2HMFWQKdzaSwX8Khlsm91IFPp/71oJCpvl7R99kEUfXH8g6fojXuiOGMyZ 7O9HryxY+0LThVBII9wg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1prItu-001HT8-1i; Tue, 25 Apr 2023 13:40:10 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1prItr-001HSP-1o for linux-arm-kernel@lists.infradead.org; Tue, 25 Apr 2023 13:40:09 +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 4CC6A4B3; Tue, 25 Apr 2023 06:40:46 -0700 (PDT) Received: from FVFF77S0Q05N.cambridge.arm.com (FVFF77S0Q05N.cambridge.arm.com [10.1.38.148]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6C1123F64C; Tue, 25 Apr 2023 06:40:00 -0700 (PDT) Date: Tue, 25 Apr 2023 14:39:51 +0100 From: Mark Rutland To: Youngmin Nam Cc: catalin.marinas@arm.com, will@kernel.org, anshuman.khandual@arm.com, broonie@kernel.org, alexandru.elisei@arm.com, ardb@kernel.org, linux-arm-kernel@lists.infradead.org, hy50.seo@samsung.com, andreyknvl@gmail.com, maz@kernel.org, kasan-dev , Dmitry Vyukov , d7271.choe@samsung.com Subject: Re: [PATCH] arm64: set __exception_irq_entry with __irq_entry as a default Message-ID: References: <20230424010436.779733-1-youngmin.nam@samsung.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230425_064007_721250_7080C327 X-CRM114-Status: GOOD ( 45.23 ) 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 Tue, Apr 25, 2023 at 11:31:31AM +0900, Youngmin Nam wrote: > On Mon, Apr 24, 2023 at 02:08:14PM +0100, Mark Rutland wrote: > > On Mon, Apr 24, 2023 at 02:09:05PM +0200, Dmitry Vyukov wrote: > > > On Mon, 24 Apr 2023 at 13:01, Mark Rutland wrote: > > > > > > > > On Mon, Apr 24, 2023 at 10:04:36AM +0900, Youngmin Nam wrote: > > > > > filter_irq_stacks() is supposed to cut entries which are related irq entries > > > > > from its call stack. > > > > > And in_irqentry_text() which is called by filter_irq_stacks() > > > > > uses __irqentry_text_start/end symbol to find irq entries in callstack. > > > > > > > > > > But it doesn't work correctly as without "CONFIG_FUNCTION_GRAPH_TRACER", > > > > > arm64 kernel doesn't include gic_handle_irq which is entry point of arm64 irq > > > > > between __irqentry_text_start and __irqentry_text_end as we discussed in below link. > > > > > > > > TBH, the __irqentry_text annotations don't make much sense, and I'd love to > > > > remove them. > > > > > > > > The irqchip handlers are not the actual exception entry points, and we invoke a > > > > fair amount of code between those and the actual IRQ handlers (e.g. to map from > > > > the irq domain to the actual hander, which might involve poking chained irqchip > > > > handlers), so it doesn't make much sense for the irqchip handlers to be > > > > special. > > > > > > > > > https://lore.kernel.org/all/CACT4Y+aReMGLYua2rCLHgFpS9io5cZC04Q8GLs-uNmrn1ezxYQ@mail.gmail.com/#t > > > > > > > > > > This problem can makes unintentional deep call stack entries especially > > > > > in KASAN enabled situation as below. > > > > > > > > What exactly does KASAN need here? Is this just to limit the depth of the > > > > trace? > > > > > > No, it's not just depth. Any uses of stack depot need stable > > > repeatable traces, so that they are deduplicated well. For irq stacks > > > it means removing the random part where the interrupt is delivered. > > > Otherwise stack depot grows without limits and overflows. > > Hi Dmitry Vyukov. > Thanks for your additional comments. > > > > > Sure -- you want to filter out the non-deterministic context that the interrupt > > was taken *from*. > > > > > We don't need the exact entry point for this. A frame "close enough" > > > may work well if there are no memory allocations/frees skipped. > > > > With that in mind, I think what we should do is cut this at the instant we > > enter the exception; for the trace below that would be el1h_64_irq. I've added > > some line spacing there to make it stand out. > > > > That would mean that we'd have three entry points that an interrupt trace might > > start from: > > > > * el1h_64_irq() > > * el0t_64_irq() > > * el0t_32_irq() > > > > Hi Mark. > Thanks for your kind review. > > If I understand your intention corretly, I should add "__irq_entry" > to C function of irq_handler as below. I'd meant something like the below, marking the assembly (as x86 does) rather than the C code. I'll try to sort that out and send a proper patch series after -rc1. Thanks, Mark. ---->8---- >From 7e54be3ea2420af348c710afab743b94ced72881 Mon Sep 17 00:00:00 2001 From: Mark Rutland Date: Tue, 25 Apr 2023 14:13:39 +0100 Subject: [PATCH] WIP: arm64: entry: handle irqentry consistently Follow the example of x86 in commit: f0178fc01fe46bab ("x86/entry: Unbreak __irqentry_text_start/end magic") ... and consistently treat the asm IRQ/FIQ entry points as irqentry, and nothing else. TODO: * Explain stackdepot details * Explain why dropping __kprobes is fine * Explain why placing entry asm in .irqentry.text is fine. * Explain why we don't need to mark ret_to_* or the actual vector * Check how this works for ftrace Signed-off-by: Mark Rutland --- arch/arm64/include/asm/exception.h | 10 +++++----- arch/arm64/include/asm/irq.h | 6 ++++++ arch/arm64/kernel/entry.S | 20 +++++++++++--------- drivers/irqchip/irq-dw-apb-ictl.c | 4 +++- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h index 92963f98afece..6c74a8a3aad99 100644 --- a/arch/arm64/include/asm/exception.h +++ b/arch/arm64/include/asm/exception.h @@ -13,11 +13,11 @@ #include -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -#define __exception_irq_entry __irq_entry -#else -#define __exception_irq_entry __kprobes -#endif +/* + * irqchip drivers shared with arch/arm mark IRQ handlers with + * __exception_irq_entry. Make this a NOP for arm64. + */ +#define __exception_irq_entry static inline unsigned long disr_to_esr(u64 disr) { diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h index fac08e18bcd51..a1af8fafc6cb5 100644 --- a/arch/arm64/include/asm/irq.h +++ b/arch/arm64/include/asm/irq.h @@ -6,6 +6,12 @@ #include +/* + * The irq entry code is in assembly. Make the build fail if + * something moves a C function into the __irq_entry section. + */ +#define __irq_entry __invalid_section + struct pt_regs; int set_handle_irq(void (*handle_irq)(struct pt_regs *)); diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S index ab2a6e33c0528..a66cd0ce79956 100644 --- a/arch/arm64/kernel/entry.S +++ b/arch/arm64/kernel/entry.S @@ -562,7 +562,8 @@ SYM_CODE_END(__bad_stack) #endif /* CONFIG_VMAP_STACK */ - .macro entry_handler el:req, ht:req, regsize:req, label:req + .macro entry_handler el:req, ht:req, regsize:req, label:req, section = ".entry.text" + .pushsection \section, "ax" SYM_CODE_START_LOCAL(el\el\ht\()_\regsize\()_\label) kernel_entry \el, \regsize mov x0, sp @@ -573,29 +574,30 @@ SYM_CODE_START_LOCAL(el\el\ht\()_\regsize\()_\label) b ret_to_kernel .endif SYM_CODE_END(el\el\ht\()_\regsize\()_\label) + .popsection .endm /* * Early exception handlers */ entry_handler 1, t, 64, sync - entry_handler 1, t, 64, irq - entry_handler 1, t, 64, fiq + entry_handler 1, t, 64, irq, ".irqentry.text" + entry_handler 1, t, 64, fiq, ".irqentry.text" entry_handler 1, t, 64, error entry_handler 1, h, 64, sync - entry_handler 1, h, 64, irq - entry_handler 1, h, 64, fiq + entry_handler 1, h, 64, irq, ".irqentry.text" + entry_handler 1, h, 64, fiq, ".irqentry.text" entry_handler 1, h, 64, error entry_handler 0, t, 64, sync - entry_handler 0, t, 64, irq - entry_handler 0, t, 64, fiq + entry_handler 0, t, 64, irq, ".irqentry.text" + entry_handler 0, t, 64, fiq, ".irqentry.text" entry_handler 0, t, 64, error entry_handler 0, t, 32, sync - entry_handler 0, t, 32, irq - entry_handler 0, t, 32, fiq + entry_handler 0, t, 32, irq, ".irqentry.text" + entry_handler 0, t, 32, fiq, ".irqentry.text" entry_handler 0, t, 32, error SYM_CODE_START_LOCAL(ret_to_kernel) diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c index d5c1c750c8d2d..ad315bdfc3ef6 100644 --- a/drivers/irqchip/irq-dw-apb-ictl.c +++ b/drivers/irqchip/irq-dw-apb-ictl.c @@ -19,6 +19,8 @@ #include #include +#include + #define APB_INT_ENABLE_L 0x00 #define APB_INT_ENABLE_H 0x04 #define APB_INT_MASK_L 0x08 @@ -30,7 +32,7 @@ /* irq domain of the primary interrupt controller. */ static struct irq_domain *dw_apb_ictl_irq_domain; -static void __irq_entry dw_apb_ictl_handle_irq(struct pt_regs *regs) +static void __exception_irq_entry dw_apb_ictl_handle_irq(struct pt_regs *regs) { struct irq_domain *d = dw_apb_ictl_irq_domain; int n; -- 2.30.2 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel