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 X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6FFA2C4332B for ; Fri, 5 Mar 2021 17:08:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5258965081 for ; Fri, 5 Mar 2021 17:08:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231400AbhCERHw (ORCPT ); Fri, 5 Mar 2021 12:07:52 -0500 Received: from mail.kernel.org ([198.145.29.99]:45588 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231387AbhCERH1 (ORCPT ); Fri, 5 Mar 2021 12:07:27 -0500 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DBEE46508B; Fri, 5 Mar 2021 17:07:26 +0000 (UTC) Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94) (envelope-from ) id 1lIDvA-00HXB9-HO; Fri, 05 Mar 2021 17:07:24 +0000 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Fri, 05 Mar 2021 17:07:24 +0000 From: Marc Zyngier To: Sami Tolvanen Cc: Ard Biesheuvel , Fangrui Song , Nick Desaulniers , James Morse , Nathan Chancellor , Kees Cook , Julien Thierry , Suzuki K Poulose , Catalin Marinas , Will Deacon , linux-arm-kernel , kvmarm , LKML Subject: Re: [PATCH] KVM: arm64: Disable LTO in hyp In-Reply-To: References: <20210304184544.2014171-1-samitolvanen@google.com> <87k0qmzq5u.wl-maz@kernel.org> <878s72sgwt.wl-maz@kernel.org> <87im65zvb9.wl-maz@kernel.org> User-Agent: Roundcube Webmail/1.4.11 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: samitolvanen@google.com, ardb@kernel.org, maskray@google.com, ndesaulniers@google.com, james.morse@arm.com, nathan@kernel.org, keescook@chromium.org, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2021-03-05 16:55, Sami Tolvanen wrote: > On Fri, Mar 5, 2021 at 6:22 AM Ard Biesheuvel wrote: >> >> On Fri, 5 Mar 2021 at 12:38, Marc Zyngier wrote: >> > >> > On Fri, 05 Mar 2021 02:38:17 +0000, >> > Sami Tolvanen wrote: >> > > >> > > On Thu, Mar 4, 2021 at 2:34 PM Sami Tolvanen wrote: >> > > > >> > > > On Thu, Mar 4, 2021 at 2:17 PM Marc Zyngier wrote: >> > > > > >> > > > > On Thu, 04 Mar 2021 21:25:41 +0000, >> > > > > Sami Tolvanen wrote: >> > >> > [...] >> > >> > > > > > I assume hyp_panic() ends up being placed too far from __guest_enter() >> > > > > > when the kernel is large enough. Possibly something to do with LLVM >> > > > > > always splitting functions into separate sections with LTO. I'm not >> > > > > > sure why the linker cannot shuffle things around to make everyone >> > > > > > happy in this case, but I confirmed that this patch also fixes the >> > > > > > build issue for me: >> > > > > > >> > > > > > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c >> > > > > > index af8e940d0f03..128197b7c794 100644 >> > > > > > --- a/arch/arm64/kvm/hyp/vhe/switch.c >> > > > > > +++ b/arch/arm64/kvm/hyp/vhe/switch.c >> > > > > > @@ -214,7 +214,7 @@ static void __hyp_call_panic(u64 spsr, u64 elr, u64 par) >> > > > > > } >> > > > > > NOKPROBE_SYMBOL(__hyp_call_panic); >> > > > > > >> > > > > > -void __noreturn hyp_panic(void) >> > > > > > +void __noreturn hyp_panic(void) __section(".text") >> > > > > > { >> > > > > > u64 spsr = read_sysreg_el2(SYS_SPSR); >> > > > > > u64 elr = read_sysreg_el2(SYS_ELR); >> > > > > > >> > > > > >> > > > > We're getting into black-magic territory here. Why wouldn't hyp_panic >> > > > > be in the .text section already? >> > > > >> > > > It's not quite black magic. LLVM essentially flips on >> > > > -ffunction-sections with LTO and therefore, hyp_panic() will be in >> > > > .text.hyp_panic in vmlinux.o, while __guest_enter() will be in .text. >> > > > Everything ends up in .text when we link vmlinux, of course. >> > > > >> > > > $ readelf --sections vmlinux.o | grep hyp_panic >> > > > [3936] .text.hyp_panic PROGBITS 0000000000000000 004b56e4 >> > > >> > > Note that disabling LTO here has essentially the same effect as using >> > > __section(".text"). It stops the compiler from splitting these >> > > functions into .text.* sections and makes it less likely that >> > > hyp_panic() ends up too far away from __guest_enter(). >> > > >> > > If neither of these workarounds sound appealing, I suppose we could >> > > alternatively change hyp/entry.S to use adr_l for hyp_panic. Thoughts? >> > >> > That would be an actual fix instead of a workaround, as it would >> > remove existing assumptions about the relative locations of the two >> > objects. I guess you need to fix both instances with something such >> > as: >> > >> > diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S >> > index b0afad7a99c6..a43e1f7ee354 100644 >> > --- a/arch/arm64/kvm/hyp/entry.S >> > +++ b/arch/arm64/kvm/hyp/entry.S >> > @@ -85,8 +85,10 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) >> > >> > // If the hyp context is loaded, go straight to hyp_panic >> > get_loaded_vcpu x0, x1 >> > - cbz x0, hyp_panic >> > - >> > + cbnz x0, 1f >> > + adr_l x0, hyp_panic >> > + br x0 >> > +1: >> >> Agree with replacing the conditional branches that refer to external >> symbols: the compiler never emits those, for the reason we are seeing >> here, i.e., the range is simply insufficient. >> >> But let's just use 'b hyp_panic' instead, no? > > Alright, this seems to work for me: > > diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S > index b0afad7a99c6..c62265951467 100644 > --- a/arch/arm64/kvm/hyp/entry.S > +++ b/arch/arm64/kvm/hyp/entry.S > @@ -85,8 +85,10 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) > > // If the hyp context is loaded, go straight to hyp_panic > get_loaded_vcpu x0, x1 > - cbz x0, hyp_panic > + cbnz x0, 1f > + b hyp_panic > > +1: > // The hyp context is saved so make sure it is restored to > allow > // hyp_panic to run at hyp and, subsequently, panic to run in > the host. > // This makes use of __guest_exit to avoid duplication but sets > the > @@ -94,7 +96,7 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) > // current state is saved to the guest context but it will only > be > // accurate if the guest had been completely restored. > adr_this_cpu x0, kvm_hyp_ctxt, x1 > - adr x1, hyp_panic > + adr_l x1, hyp_panic > str x1, [x0, #CPU_XREG_OFFSET(30)] > > get_vcpu_ptr x1, x0 > > But when I say work, I mean this fixes the allmodconfig build with > LTO, and my kernel boots at EL2. I don't actually have a way to > properly test KVM on arm64. If nobody sees obvious issues here, I can > send a proper patch a bit later. Please do, it is "obviously correct"! ;-) Thanks, M. -- Jazz is not dead. It just smells funny... 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 X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8D2B6C433E0 for ; Fri, 5 Mar 2021 17:07:33 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id D59786508D for ; Fri, 5 Mar 2021 17:07:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D59786508D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 6E8844B565; Fri, 5 Mar 2021 12:07:32 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7RGax3LA7rKs; Fri, 5 Mar 2021 12:07:31 -0500 (EST) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 2A8B74B574; Fri, 5 Mar 2021 12:07:31 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 880C54B565 for ; Fri, 5 Mar 2021 12:07:29 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ob8rjcqiW63n for ; Fri, 5 Mar 2021 12:07:28 -0500 (EST) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 2AFB64B559 for ; Fri, 5 Mar 2021 12:07:28 -0500 (EST) Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DBEE46508B; Fri, 5 Mar 2021 17:07:26 +0000 (UTC) Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94) (envelope-from ) id 1lIDvA-00HXB9-HO; Fri, 05 Mar 2021 17:07:24 +0000 MIME-Version: 1.0 Date: Fri, 05 Mar 2021 17:07:24 +0000 From: Marc Zyngier To: Sami Tolvanen Subject: Re: [PATCH] KVM: arm64: Disable LTO in hyp In-Reply-To: References: <20210304184544.2014171-1-samitolvanen@google.com> <87k0qmzq5u.wl-maz@kernel.org> <878s72sgwt.wl-maz@kernel.org> <87im65zvb9.wl-maz@kernel.org> User-Agent: Roundcube Webmail/1.4.11 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: samitolvanen@google.com, ardb@kernel.org, maskray@google.com, ndesaulniers@google.com, james.morse@arm.com, nathan@kernel.org, keescook@chromium.org, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: Kees Cook , Fangrui Song , Catalin Marinas , Nick Desaulniers , LKML , kvmarm , Nathan Chancellor , linux-arm-kernel , Will Deacon , Ard Biesheuvel X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On 2021-03-05 16:55, Sami Tolvanen wrote: > On Fri, Mar 5, 2021 at 6:22 AM Ard Biesheuvel wrote: >> >> On Fri, 5 Mar 2021 at 12:38, Marc Zyngier wrote: >> > >> > On Fri, 05 Mar 2021 02:38:17 +0000, >> > Sami Tolvanen wrote: >> > > >> > > On Thu, Mar 4, 2021 at 2:34 PM Sami Tolvanen wrote: >> > > > >> > > > On Thu, Mar 4, 2021 at 2:17 PM Marc Zyngier wrote: >> > > > > >> > > > > On Thu, 04 Mar 2021 21:25:41 +0000, >> > > > > Sami Tolvanen wrote: >> > >> > [...] >> > >> > > > > > I assume hyp_panic() ends up being placed too far from __guest_enter() >> > > > > > when the kernel is large enough. Possibly something to do with LLVM >> > > > > > always splitting functions into separate sections with LTO. I'm not >> > > > > > sure why the linker cannot shuffle things around to make everyone >> > > > > > happy in this case, but I confirmed that this patch also fixes the >> > > > > > build issue for me: >> > > > > > >> > > > > > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c >> > > > > > index af8e940d0f03..128197b7c794 100644 >> > > > > > --- a/arch/arm64/kvm/hyp/vhe/switch.c >> > > > > > +++ b/arch/arm64/kvm/hyp/vhe/switch.c >> > > > > > @@ -214,7 +214,7 @@ static void __hyp_call_panic(u64 spsr, u64 elr, u64 par) >> > > > > > } >> > > > > > NOKPROBE_SYMBOL(__hyp_call_panic); >> > > > > > >> > > > > > -void __noreturn hyp_panic(void) >> > > > > > +void __noreturn hyp_panic(void) __section(".text") >> > > > > > { >> > > > > > u64 spsr = read_sysreg_el2(SYS_SPSR); >> > > > > > u64 elr = read_sysreg_el2(SYS_ELR); >> > > > > > >> > > > > >> > > > > We're getting into black-magic territory here. Why wouldn't hyp_panic >> > > > > be in the .text section already? >> > > > >> > > > It's not quite black magic. LLVM essentially flips on >> > > > -ffunction-sections with LTO and therefore, hyp_panic() will be in >> > > > .text.hyp_panic in vmlinux.o, while __guest_enter() will be in .text. >> > > > Everything ends up in .text when we link vmlinux, of course. >> > > > >> > > > $ readelf --sections vmlinux.o | grep hyp_panic >> > > > [3936] .text.hyp_panic PROGBITS 0000000000000000 004b56e4 >> > > >> > > Note that disabling LTO here has essentially the same effect as using >> > > __section(".text"). It stops the compiler from splitting these >> > > functions into .text.* sections and makes it less likely that >> > > hyp_panic() ends up too far away from __guest_enter(). >> > > >> > > If neither of these workarounds sound appealing, I suppose we could >> > > alternatively change hyp/entry.S to use adr_l for hyp_panic. Thoughts? >> > >> > That would be an actual fix instead of a workaround, as it would >> > remove existing assumptions about the relative locations of the two >> > objects. I guess you need to fix both instances with something such >> > as: >> > >> > diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S >> > index b0afad7a99c6..a43e1f7ee354 100644 >> > --- a/arch/arm64/kvm/hyp/entry.S >> > +++ b/arch/arm64/kvm/hyp/entry.S >> > @@ -85,8 +85,10 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) >> > >> > // If the hyp context is loaded, go straight to hyp_panic >> > get_loaded_vcpu x0, x1 >> > - cbz x0, hyp_panic >> > - >> > + cbnz x0, 1f >> > + adr_l x0, hyp_panic >> > + br x0 >> > +1: >> >> Agree with replacing the conditional branches that refer to external >> symbols: the compiler never emits those, for the reason we are seeing >> here, i.e., the range is simply insufficient. >> >> But let's just use 'b hyp_panic' instead, no? > > Alright, this seems to work for me: > > diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S > index b0afad7a99c6..c62265951467 100644 > --- a/arch/arm64/kvm/hyp/entry.S > +++ b/arch/arm64/kvm/hyp/entry.S > @@ -85,8 +85,10 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) > > // If the hyp context is loaded, go straight to hyp_panic > get_loaded_vcpu x0, x1 > - cbz x0, hyp_panic > + cbnz x0, 1f > + b hyp_panic > > +1: > // The hyp context is saved so make sure it is restored to > allow > // hyp_panic to run at hyp and, subsequently, panic to run in > the host. > // This makes use of __guest_exit to avoid duplication but sets > the > @@ -94,7 +96,7 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) > // current state is saved to the guest context but it will only > be > // accurate if the guest had been completely restored. > adr_this_cpu x0, kvm_hyp_ctxt, x1 > - adr x1, hyp_panic > + adr_l x1, hyp_panic > str x1, [x0, #CPU_XREG_OFFSET(30)] > > get_vcpu_ptr x1, x0 > > But when I say work, I mean this fixes the allmodconfig build with > LTO, and my kernel boots at EL2. I don't actually have a way to > properly test KVM on arm64. If nobody sees obvious issues here, I can > send a proper patch a bit later. Please do, it is "obviously correct"! ;-) Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm 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 X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 275F7C433DB for ; Fri, 5 Mar 2021 17:09:20 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B975C64FEB for ; Fri, 5 Mar 2021 17:09:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B975C64FEB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:Cc:To:From :Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=rdNGQkhM0MrBP9TS20uTJibLJ8jUOqAt1fCs1c+3QXA=; b=h1XKSqBEEfQ5AI/Egj1ulvNuE yfX35cC90gZ5VDMay1ECjlIjm6LrytWIu+qIdnlOKXCTjIc+Ub3Y3pVVjb58xFltzgnO45oXvtx3b NpnVb2s1L69MvLrLKIBw26m2WYfE8gv6qACsRx8iPlKfM+SnHXEWo4rUBlLWmKXXB6rij4Rcj+JlP aDxYGMsK1f6caLcxqdtKVz6aoLqbqHJ+IkgdR+mPaRcoXj0VseI+1EV6UGWU2UnjHcm5UQ4xtpGAa 0TFByhB7a6TdLOPUgmGqMZM6eMBZY5L7XlZkzbOaUuUyzlsMeXf9W/fdFm3oHtDV/T1Ql2s4QyUZR XUZi1W64Q==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lIDvN-00FjO8-IH; Fri, 05 Mar 2021 17:07:37 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lIDvF-00FjMg-2D for linux-arm-kernel@lists.infradead.org; Fri, 05 Mar 2021 17:07:31 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DBEE46508B; Fri, 5 Mar 2021 17:07:26 +0000 (UTC) Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94) (envelope-from ) id 1lIDvA-00HXB9-HO; Fri, 05 Mar 2021 17:07:24 +0000 MIME-Version: 1.0 Date: Fri, 05 Mar 2021 17:07:24 +0000 From: Marc Zyngier To: Sami Tolvanen Cc: Ard Biesheuvel , Fangrui Song , Nick Desaulniers , James Morse , Nathan Chancellor , Kees Cook , Julien Thierry , Suzuki K Poulose , Catalin Marinas , Will Deacon , linux-arm-kernel , kvmarm , LKML Subject: Re: [PATCH] KVM: arm64: Disable LTO in hyp In-Reply-To: References: <20210304184544.2014171-1-samitolvanen@google.com> <87k0qmzq5u.wl-maz@kernel.org> <878s72sgwt.wl-maz@kernel.org> <87im65zvb9.wl-maz@kernel.org> User-Agent: Roundcube Webmail/1.4.11 Message-ID: X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: samitolvanen@google.com, ardb@kernel.org, maskray@google.com, ndesaulniers@google.com, james.morse@arm.com, nathan@kernel.org, keescook@chromium.org, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com, catalin.marinas@arm.com, will@kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210305_170729_945602_3116B12D X-CRM114-Status: GOOD ( 41.79 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2021-03-05 16:55, Sami Tolvanen wrote: > On Fri, Mar 5, 2021 at 6:22 AM Ard Biesheuvel wrote: >> >> On Fri, 5 Mar 2021 at 12:38, Marc Zyngier wrote: >> > >> > On Fri, 05 Mar 2021 02:38:17 +0000, >> > Sami Tolvanen wrote: >> > > >> > > On Thu, Mar 4, 2021 at 2:34 PM Sami Tolvanen wrote: >> > > > >> > > > On Thu, Mar 4, 2021 at 2:17 PM Marc Zyngier wrote: >> > > > > >> > > > > On Thu, 04 Mar 2021 21:25:41 +0000, >> > > > > Sami Tolvanen wrote: >> > >> > [...] >> > >> > > > > > I assume hyp_panic() ends up being placed too far from __guest_enter() >> > > > > > when the kernel is large enough. Possibly something to do with LLVM >> > > > > > always splitting functions into separate sections with LTO. I'm not >> > > > > > sure why the linker cannot shuffle things around to make everyone >> > > > > > happy in this case, but I confirmed that this patch also fixes the >> > > > > > build issue for me: >> > > > > > >> > > > > > diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c >> > > > > > index af8e940d0f03..128197b7c794 100644 >> > > > > > --- a/arch/arm64/kvm/hyp/vhe/switch.c >> > > > > > +++ b/arch/arm64/kvm/hyp/vhe/switch.c >> > > > > > @@ -214,7 +214,7 @@ static void __hyp_call_panic(u64 spsr, u64 elr, u64 par) >> > > > > > } >> > > > > > NOKPROBE_SYMBOL(__hyp_call_panic); >> > > > > > >> > > > > > -void __noreturn hyp_panic(void) >> > > > > > +void __noreturn hyp_panic(void) __section(".text") >> > > > > > { >> > > > > > u64 spsr = read_sysreg_el2(SYS_SPSR); >> > > > > > u64 elr = read_sysreg_el2(SYS_ELR); >> > > > > > >> > > > > >> > > > > We're getting into black-magic territory here. Why wouldn't hyp_panic >> > > > > be in the .text section already? >> > > > >> > > > It's not quite black magic. LLVM essentially flips on >> > > > -ffunction-sections with LTO and therefore, hyp_panic() will be in >> > > > .text.hyp_panic in vmlinux.o, while __guest_enter() will be in .text. >> > > > Everything ends up in .text when we link vmlinux, of course. >> > > > >> > > > $ readelf --sections vmlinux.o | grep hyp_panic >> > > > [3936] .text.hyp_panic PROGBITS 0000000000000000 004b56e4 >> > > >> > > Note that disabling LTO here has essentially the same effect as using >> > > __section(".text"). It stops the compiler from splitting these >> > > functions into .text.* sections and makes it less likely that >> > > hyp_panic() ends up too far away from __guest_enter(). >> > > >> > > If neither of these workarounds sound appealing, I suppose we could >> > > alternatively change hyp/entry.S to use adr_l for hyp_panic. Thoughts? >> > >> > That would be an actual fix instead of a workaround, as it would >> > remove existing assumptions about the relative locations of the two >> > objects. I guess you need to fix both instances with something such >> > as: >> > >> > diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S >> > index b0afad7a99c6..a43e1f7ee354 100644 >> > --- a/arch/arm64/kvm/hyp/entry.S >> > +++ b/arch/arm64/kvm/hyp/entry.S >> > @@ -85,8 +85,10 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) >> > >> > // If the hyp context is loaded, go straight to hyp_panic >> > get_loaded_vcpu x0, x1 >> > - cbz x0, hyp_panic >> > - >> > + cbnz x0, 1f >> > + adr_l x0, hyp_panic >> > + br x0 >> > +1: >> >> Agree with replacing the conditional branches that refer to external >> symbols: the compiler never emits those, for the reason we are seeing >> here, i.e., the range is simply insufficient. >> >> But let's just use 'b hyp_panic' instead, no? > > Alright, this seems to work for me: > > diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S > index b0afad7a99c6..c62265951467 100644 > --- a/arch/arm64/kvm/hyp/entry.S > +++ b/arch/arm64/kvm/hyp/entry.S > @@ -85,8 +85,10 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) > > // If the hyp context is loaded, go straight to hyp_panic > get_loaded_vcpu x0, x1 > - cbz x0, hyp_panic > + cbnz x0, 1f > + b hyp_panic > > +1: > // The hyp context is saved so make sure it is restored to > allow > // hyp_panic to run at hyp and, subsequently, panic to run in > the host. > // This makes use of __guest_exit to avoid duplication but sets > the > @@ -94,7 +96,7 @@ SYM_INNER_LABEL(__guest_exit_panic, SYM_L_GLOBAL) > // current state is saved to the guest context but it will only > be > // accurate if the guest had been completely restored. > adr_this_cpu x0, kvm_hyp_ctxt, x1 > - adr x1, hyp_panic > + adr_l x1, hyp_panic > str x1, [x0, #CPU_XREG_OFFSET(30)] > > get_vcpu_ptr x1, x0 > > But when I say work, I mean this fixes the allmodconfig build with > LTO, and my kernel boots at EL2. I don't actually have a way to > properly test KVM on arm64. If nobody sees obvious issues here, I can > send a proper patch a bit later. Please do, it is "obviously correct"! ;-) Thanks, M. -- Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel