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 A92B4C433EF for ; Mon, 6 Dec 2021 16:32:14 +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=PiDXGwVT2+dUKLl3yzasFeWmrrbLC6LPBGIkByRA1JM=; b=mNYuEcQk/eA7qz Bunen0BpSJz59yUJOp23WxMKQ4bfTzRjeWgsJysn3km8e51tyNwdZW/DJNox6UkHj248f+to4vGuK XEBoEdi5cz9IeCZw94asV+Xy+GqrImq2W/iEAAFqVNx6a/6hfQ7JurJKx9p3bA0+++ItaiCKlrx/4 jRbPkh2cA5+v2h2RixQPwxCagcR7+0UfIHRTNTDrVxbxAKCEED7sKssUhNQlxNj+3ubwxhqeIQNQ8 AbvGVtjD5FSKKHtkhgKM2s0JFxkdvdG6g80cgISmkHXqSoTcbnRf+7I4TAMzSLKt8OwI+9VU5sHIT hpT1RNXK4A7BHzUg+NtA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1muGt8-004mlY-Oe; Mon, 06 Dec 2021 16:30:50 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1muGt4-004mkr-N8 for linux-arm-kernel@lists.infradead.org; Mon, 06 Dec 2021 16:30:48 +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 1DCFD12FC; Mon, 6 Dec 2021 08:30:45 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.65.75]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2587B3FA00; Mon, 6 Dec 2021 08:30:44 -0800 (PST) Date: Mon, 6 Dec 2021 16:30:41 +0000 From: Mark Rutland To: Mark Brown Cc: Catalin Marinas , Will Deacon , linux-arm-kernel@lists.infradead.org, Ard Biesheuvel Subject: Re: [PATCH] arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE Message-ID: References: <20211203130335.84733-1-broonie@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211203130335.84733-1-broonie@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211206_083046_870747_AB0170CA X-CRM114-Status: GOOD ( 23.57 ) 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 Fri, Dec 03, 2021 at 01:03:35PM +0000, Mark Brown wrote: > A couple of SYM_CODE sections have added usage of BTI_C which is > currently only defined when building for BTI. This means that the > users have ugly ifdefs for the case where BTI is disabled so let's > provide an empty definition in that case and remove the ifdefs. > > Signed-off-by: Mark Brown > --- > arch/arm64/include/asm/linkage.h | 4 ++++ > arch/arm64/kernel/entry-ftrace.S | 4 ---- > arch/arm64/lib/kasan_sw_tags.S | 2 -- > 3 files changed, 4 insertions(+), 6 deletions(-) Looking around, there are other places that open-code `hint 34`, e.g. arch/arm64/crypto/aes-modes.S. Those are unconditional, so we should probably figure out whether we want those to be conditional (or if we're happy to make the other cases similarly unconditional). I'd argue we should probably place BTIs in assembly unconditionally, on the assumption that they shouldn't have an measureable performance impact in practice (as we're already assuming that when CONFIG_ARM64_BTI_KERNEL is selected anyhow). Thoughts? Regardless, I reckon we should probably define a `bti` macro centrally in arch/arm64/include/asm/assembler.h, something like: | /* | * Branch Target Identifier | */ | .macro bti, targets | .equ .L__bti_targets_c, 1 | .equ .L__bti_targets_j, 2 | .equ .L__bti_targets_jc,3 | .if .L__bti_targets_\targets == .L__bti_targets_c | hint #34 | .elseif .L__bti_targets_\targets == .L__bti_targets_j | hint #36 | .elseif .L__bti_targets_\targets == .L__bti_targets_jc | hint #38 | .else | .error "Unsupported BTI targets '\targets\()'" | .endif | .endm | The `.equ` gunk is because there's no `.elseifc`, and without that we have to have a chain of `.endif` for each `.ifc`, and can't produce a warning reliably. That seems to do the right thing: | [mark@lakrids:~/src/linux]% git diff -- arch/arm64/kernel/head.S | diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S | index 6a98f1a38c29..f6968563bab8 100644 | --- a/arch/arm64/kernel/head.S | +++ b/arch/arm64/kernel/head.S | @@ -89,6 +89,10 @@ | * x24 __primary_switch() .. relocate_kernel() current RELR displacement | */ | SYM_CODE_START(primary_entry) | + bti c | + bti j | + bti jc | + bti no | bl preserve_boot_args | bl init_kernel_el // w0=cpu_boot_mode | adrp x23, __PHYS_OFFSET | [mark@lakrids:~/src/linux]% usekorg 11.1.0 make ARCH=arm64 CROSS_COMPILE=aarch64-linux- arch/arm64/kernel/head.o | Makefile:1811: warning: overriding recipe for target 'arch/arm64/kernel/head.o' | Makefile:1167: warning: ignoring old recipe for target 'arch/arm64/kernel/head.o' | CALL scripts/checksyscalls.sh | CALL scripts/atomic/check-atomics.sh | AS arch/arm64/kernel/head.o | arch/arm64/kernel/head.S: Assembler messages: | arch/arm64/kernel/head.S:95: Error: Unsupported BTI targets 'no' | make[2]: *** [scripts/Makefile.build:388: arch/arm64/kernel/head.o] Error 1 | make[1]: *** [scripts/Makefile.build:549: arch/arm64/kernel] Error 2 | make: *** [Makefile:1846: arch/arm64] Error 2 > diff --git a/arch/arm64/include/asm/linkage.h b/arch/arm64/include/asm/linkage.h > index 9906541a6861..9c70136d7f94 100644 > --- a/arch/arm64/include/asm/linkage.h > +++ b/arch/arm64/include/asm/linkage.h > @@ -42,6 +42,10 @@ > SYM_START(name, SYM_L_WEAK, SYM_A_NONE) \ > BTI_C > > +#else > + > +#define BTI_C > + > #endif Can we simplify the ifdeffery so that we only conditionally define BTI_C, and unconditionally define the sites using it? That way if there's a problem with any of the users that will consistently show up in testing, regardless of whether CONFIG_ARM64_BTI_KERNEL is selected. e.g. we can make this: | #ifdef CONFIG_ARM64_BTI_KERNEL | #define BTI_C hint #34 ; | #else | #define BTI_C | #endif | | #define SYM_FUNC_START(name) \ | SYM_START(name, SYM_L_GLOBAL, SYM_A_ALIGN) \ | BTI_C | | ... ... or if we define an unconditional `bti` macro: | #define SYM_FUNC_START(name) \ | SYM_START(name, SYM_L_GLOBAL, SYM_A_ALIGN) \ | bti c | | ... Thanks, Mark. > /* > diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S > index 8cf970d219f5..46a2de864794 100644 > --- a/arch/arm64/kernel/entry-ftrace.S > +++ b/arch/arm64/kernel/entry-ftrace.S > @@ -77,17 +77,13 @@ > .endm > > SYM_CODE_START(ftrace_regs_caller) > -#ifdef BTI_C > BTI_C > -#endif > ftrace_regs_entry 1 > b ftrace_common > SYM_CODE_END(ftrace_regs_caller) > > SYM_CODE_START(ftrace_caller) > -#ifdef BTI_C > BTI_C > -#endif > ftrace_regs_entry 0 > b ftrace_common > SYM_CODE_END(ftrace_caller) > diff --git a/arch/arm64/lib/kasan_sw_tags.S b/arch/arm64/lib/kasan_sw_tags.S > index 5b04464c045e..a6d6fa2f761e 100644 > --- a/arch/arm64/lib/kasan_sw_tags.S > +++ b/arch/arm64/lib/kasan_sw_tags.S > @@ -38,9 +38,7 @@ > * incremented by 256 prior to return). > */ > SYM_CODE_START(__hwasan_tag_mismatch) > -#ifdef BTI_C > BTI_C > -#endif > add x29, sp, #232 > stp x2, x3, [sp, #8 * 2] > stp x4, x5, [sp, #8 * 4] > -- > 2.30.2 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel