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=-18.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL 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 B6ECFC433DB for ; Fri, 5 Mar 2021 16:56:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 789136507C for ; Fri, 5 Mar 2021 16:56:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231252AbhCEQ4C (ORCPT ); Fri, 5 Mar 2021 11:56:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37602 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230287AbhCEQzy (ORCPT ); Fri, 5 Mar 2021 11:55:54 -0500 Received: from mail-vs1-xe2b.google.com (mail-vs1-xe2b.google.com [IPv6:2607:f8b0:4864:20::e2b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C1E33C061574 for ; Fri, 5 Mar 2021 08:55:54 -0800 (PST) Received: by mail-vs1-xe2b.google.com with SMTP id a12so1312511vsd.3 for ; Fri, 05 Mar 2021 08:55:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NKf5oBBu2KISuo5V0ahvjDLUwLWiqpHm4voeGNA9XVQ=; b=MFc/EgDeUn5T82qMVlgRjQ8BfFaLSo8y39d7Tq6SwZJoyUiEUORWtASykVvNbxEIyF FP1J8f+/4QHcuUIRn7KvRoG1BoNPrbI5TsXRA8ViSu6CusWjgPcQfCW+Xzu8vHu6LOYr jOi/jbaJQlMBxQ8PPc1CaXyub0rqamP3K9ttcvy0W+UfRQ9XKeeqbvYX72/5dzAtnpr6 /7pMt32h3e5jH2DgdcjfdxCLx5DInK3o3UhTtsenKKiGCKFKfyzYTI8e/2jFriUSO6yL 40xvLUDycCVb4xx0SgkLpiYwK14bnT68fRLDitnsudT4vHsxFbW/obrkBY5ISbUWs5AE 2Arw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=NKf5oBBu2KISuo5V0ahvjDLUwLWiqpHm4voeGNA9XVQ=; b=T8ki4etfZioubGmwOnnGDkywuI3eLqfHr/9n2XgCQ5Yh84wcLvI913c1LXP1iPSNnx +TN7uq1ze00tf9QXLohuo49SUer83gC5+K+0Yy4lp/9alyDtLIrMaZNsZGisbic9MMt/ weJNASsH/LU+gKP8Bfd/Dqa6wDnCGXEVTJNqZ34nhmpmIY/RU68jwttai9LpMOh0YR57 sbX4dO+UCga3Sdd/dpujAOcJw6rfjuX1wXVr9W9bgsplcZpsmN/83RlQcACgnMG4z9Df psA8acLHDNFU39FrS/q9Lu2Iuedlls9mz0QsTs3MN5yc1XPVq8arqTup4aL6xVZOAFIJ cJYQ== X-Gm-Message-State: AOAM530ErhP8mwbl8T7RJkRKBhL1wgX5hoe6lX0Rrf4WjBKPY/ArLInJ tswR/eg2gbt0/pB7hEnp7XhEAXU+fInkbNBGgWw6ew== X-Google-Smtp-Source: ABdhPJzpfWrcs5DxykHEI6KERA5BOBknmIj6EemM4VWPPImoQO4fG5rdh8rsez5jUuFficqkpwK3BC3QFLegheMWHTs= X-Received: by 2002:a67:ed4a:: with SMTP id m10mr7437760vsp.14.1614963353636; Fri, 05 Mar 2021 08:55:53 -0800 (PST) MIME-Version: 1.0 References: <20210304184544.2014171-1-samitolvanen@google.com> <87k0qmzq5u.wl-maz@kernel.org> <878s72sgwt.wl-maz@kernel.org> <87im65zvb9.wl-maz@kernel.org> In-Reply-To: From: Sami Tolvanen Date: Fri, 5 Mar 2021 08:55:42 -0800 Message-ID: Subject: Re: [PATCH] KVM: arm64: Disable LTO in hyp To: Ard Biesheuvel Cc: Marc Zyngier , Fangrui Song , Nick Desaulniers , James Morse , Nathan Chancellor , Kees Cook , Julien Thierry , Suzuki K Poulose , Catalin Marinas , Will Deacon , linux-arm-kernel , kvmarm , LKML Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > // 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 > > > > which is completely untested. I wouldn't be surprised if there were > > more of these somewhere. > > > > A quick grep gives me > > $ objdump -r vmlinux.o |grep BR19 > 000000000005b6e0 R_AARCH64_CONDBR19 hyp_panic > 0000000000418e08 R_AARCH64_CONDBR19 __memcpy > 0000000000418e14 R_AARCH64_CONDBR19 __memcpy These two are in __memmove. This shouldn't be an issue, at least with LTO. > 0000000000003818 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003898 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003918 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003998 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003a18 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003a98 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003b18 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003b98 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003c10 R_AARCH64_CONDBR19 __kvm_nvhe___host_exit > 0000000000003c1c R_AARCH64_CONDBR19 __kvm_nvhe___host_exit > 00000000000064f0 R_AARCH64_CONDBR19 __kvm_nvhe_hyp_panic > 000000000000078c R_AARCH64_CONDBR19 __kvm_nvhe___kvm_handle_stub_hvc It looks like all the nvhe code is linked together into kvm_nvhe.o, so I don't think these will cause problems either. Sami 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=-8.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,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 DAD17C433E6 for ; Fri, 5 Mar 2021 18:58:39 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 47EE964F4C for ; Fri, 5 Mar 2021 18:58:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 47EE964F4C Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com 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 D70B64B27D; Fri, 5 Mar 2021 13:58:38 -0500 (EST) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@google.com 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 gci09nxMexdT; Fri, 5 Mar 2021 13:58:37 -0500 (EST) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 984654B43F; Fri, 5 Mar 2021 13:58:34 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id C24454B21E for ; Fri, 5 Mar 2021 11:55:55 -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 q0M293qn3vG1 for ; Fri, 5 Mar 2021 11:55:54 -0500 (EST) Received: from mail-vs1-f46.google.com (mail-vs1-f46.google.com [209.85.217.46]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 6C5E24B204 for ; Fri, 5 Mar 2021 11:55:54 -0500 (EST) Received: by mail-vs1-f46.google.com with SMTP id b189so1319208vsd.0 for ; Fri, 05 Mar 2021 08:55:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NKf5oBBu2KISuo5V0ahvjDLUwLWiqpHm4voeGNA9XVQ=; b=MFc/EgDeUn5T82qMVlgRjQ8BfFaLSo8y39d7Tq6SwZJoyUiEUORWtASykVvNbxEIyF FP1J8f+/4QHcuUIRn7KvRoG1BoNPrbI5TsXRA8ViSu6CusWjgPcQfCW+Xzu8vHu6LOYr jOi/jbaJQlMBxQ8PPc1CaXyub0rqamP3K9ttcvy0W+UfRQ9XKeeqbvYX72/5dzAtnpr6 /7pMt32h3e5jH2DgdcjfdxCLx5DInK3o3UhTtsenKKiGCKFKfyzYTI8e/2jFriUSO6yL 40xvLUDycCVb4xx0SgkLpiYwK14bnT68fRLDitnsudT4vHsxFbW/obrkBY5ISbUWs5AE 2Arw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=NKf5oBBu2KISuo5V0ahvjDLUwLWiqpHm4voeGNA9XVQ=; b=OOXg+yKP0gGIhLUszHHnwyl39sI5D9JlJf6h2i1O5IpsoCoK0lgOMo3JaK0hkB7FN5 5E8YQHXrzD8I5i33ajdqgBKZkyZhF98K+Y+vithKqNJaSWK5tqmhBi8nBU5BqUE1TwCs 7G7uGORb0rMtuHyyZkcXNPPAp3+5sz3E0P2R7KSR6HO2q9y+K/Fmy1PQWQ43Ap8yqxeX EY11zDZu+QtCZP1wAhW+RWFhr11Hqhpj9gJS1V2XUNYq9x1vsb5Xc6ki9rvCtoJjAmGP 1hGHYl5cGQGAS7NZ12frW5P4UaQmpZW+aI8YeVYepOPP59NBzKPiw9objDkFaBpBBC39 lmbQ== X-Gm-Message-State: AOAM530+nFMozdiJozgOSeTD9YztTB5lSqOraznQ/VFMOxf/kndc+8Pt MbVYSTbm0AnsKG4WM+c3LduTZXqg2MoQwDoxcLrgog== X-Google-Smtp-Source: ABdhPJzpfWrcs5DxykHEI6KERA5BOBknmIj6EemM4VWPPImoQO4fG5rdh8rsez5jUuFficqkpwK3BC3QFLegheMWHTs= X-Received: by 2002:a67:ed4a:: with SMTP id m10mr7437760vsp.14.1614963353636; Fri, 05 Mar 2021 08:55:53 -0800 (PST) MIME-Version: 1.0 References: <20210304184544.2014171-1-samitolvanen@google.com> <87k0qmzq5u.wl-maz@kernel.org> <878s72sgwt.wl-maz@kernel.org> <87im65zvb9.wl-maz@kernel.org> In-Reply-To: From: Sami Tolvanen Date: Fri, 5 Mar 2021 08:55:42 -0800 Message-ID: Subject: Re: [PATCH] KVM: arm64: Disable LTO in hyp To: Ard Biesheuvel X-Mailman-Approved-At: Fri, 05 Mar 2021 13:58:33 -0500 Cc: Kees Cook , Fangrui Song , Marc Zyngier , Nick Desaulniers , LKML , Nathan Chancellor , linux-arm-kernel , Catalin Marinas , Will Deacon , kvmarm 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-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu 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. > > // 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 > > > > which is completely untested. I wouldn't be surprised if there were > > more of these somewhere. > > > > A quick grep gives me > > $ objdump -r vmlinux.o |grep BR19 > 000000000005b6e0 R_AARCH64_CONDBR19 hyp_panic > 0000000000418e08 R_AARCH64_CONDBR19 __memcpy > 0000000000418e14 R_AARCH64_CONDBR19 __memcpy These two are in __memmove. This shouldn't be an issue, at least with LTO. > 0000000000003818 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003898 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003918 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003998 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003a18 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003a98 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003b18 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003b98 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003c10 R_AARCH64_CONDBR19 __kvm_nvhe___host_exit > 0000000000003c1c R_AARCH64_CONDBR19 __kvm_nvhe___host_exit > 00000000000064f0 R_AARCH64_CONDBR19 __kvm_nvhe_hyp_panic > 000000000000078c R_AARCH64_CONDBR19 __kvm_nvhe___kvm_handle_stub_hvc It looks like all the nvhe code is linked together into kvm_nvhe.o, so I don't think these will cause problems either. Sami _______________________________________________ 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=-8.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, 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 7EF1FC433E0 for ; Fri, 5 Mar 2021 16:57:46 +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 019916507C for ; Fri, 5 Mar 2021 16:57:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 019916507C Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com 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-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=3sawZEIyM7rdp3p+GjTjznUtcQRwvLHG5aF+tnrhI/g=; b=B3W+YLcdm66xFdKo+M0J0zc/P xJpeI5GWcjpX81yx0wy2lRr29rf4wPNtfFckoZW5I2oFZPDANmztGJa41JQWB63g49SbE084Dgych c8SXf0bEOu//MzjM9GgsRo1PoI5ytFAauXyWhYmiudv+v5clfrtE4kF6yUykOIq6nVapwDLcZxIxC pVwVV0SxULxtoLhx0pxIuGAfiHpwAXKreTvSAKo5WLSvhlNHjv2qnkL8iCGxZL7c8Px6a7+MfNFsJ GL3wUuDlzYWhvXrCSabbP6JlVRs7dvUTXqLU9ZvPb54NFI97uNALF8sNACI3UQpGQiFWgrao9qFQI NH2qNkPZA==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lIDkC-00FhCv-29; Fri, 05 Mar 2021 16:56:04 +0000 Received: from mail-vs1-xe29.google.com ([2607:f8b0:4864:20::e29]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lIDk3-00FhBS-KB for linux-arm-kernel@lists.infradead.org; Fri, 05 Mar 2021 16:55:59 +0000 Received: by mail-vs1-xe29.google.com with SMTP id z65so1288509vsz.12 for ; Fri, 05 Mar 2021 08:55:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NKf5oBBu2KISuo5V0ahvjDLUwLWiqpHm4voeGNA9XVQ=; b=MFc/EgDeUn5T82qMVlgRjQ8BfFaLSo8y39d7Tq6SwZJoyUiEUORWtASykVvNbxEIyF FP1J8f+/4QHcuUIRn7KvRoG1BoNPrbI5TsXRA8ViSu6CusWjgPcQfCW+Xzu8vHu6LOYr jOi/jbaJQlMBxQ8PPc1CaXyub0rqamP3K9ttcvy0W+UfRQ9XKeeqbvYX72/5dzAtnpr6 /7pMt32h3e5jH2DgdcjfdxCLx5DInK3o3UhTtsenKKiGCKFKfyzYTI8e/2jFriUSO6yL 40xvLUDycCVb4xx0SgkLpiYwK14bnT68fRLDitnsudT4vHsxFbW/obrkBY5ISbUWs5AE 2Arw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=NKf5oBBu2KISuo5V0ahvjDLUwLWiqpHm4voeGNA9XVQ=; b=LvQ6WfYnNw6y3WOME5lher2SgTs07d5lnyP5N3KLPbSutg0iGAQZgWEJ9Ix5oL4D01 zOk3abWUMVMkAyZxsUxzbHl7wS53iuYUNJZW5zAOMj74wmNnQ4UL09rWoA8EmB0//hJJ WhaykrQRFd9sLAT2TYvCqwPJxy6zWNoDoaSpItdwA1yQY2jQ/bSlM77GGyS08IYfMfyg 7Y2MrfUruxp8l28N2gmOVqaxL/v87oYqlB4WXtIEAaPIIEK3KMhFfDNFAP/i06zE7kBb 8vPlgwezYJvNrDQdwZFynsETlHcTGURo+knPIv/rjEjwi8v+Rw6aGSEhHSBuGLUcsn9R yIgw== X-Gm-Message-State: AOAM531PuBJFpTQ7f8FSKnJA8TKrGVf5R1nIGTxTQi2fcrZJASAeIZHu zTZbYMDhyOXLFD8MYrgInpY4P32p1AkWdb8b1KS2BA== X-Google-Smtp-Source: ABdhPJzpfWrcs5DxykHEI6KERA5BOBknmIj6EemM4VWPPImoQO4fG5rdh8rsez5jUuFficqkpwK3BC3QFLegheMWHTs= X-Received: by 2002:a67:ed4a:: with SMTP id m10mr7437760vsp.14.1614963353636; Fri, 05 Mar 2021 08:55:53 -0800 (PST) MIME-Version: 1.0 References: <20210304184544.2014171-1-samitolvanen@google.com> <87k0qmzq5u.wl-maz@kernel.org> <878s72sgwt.wl-maz@kernel.org> <87im65zvb9.wl-maz@kernel.org> In-Reply-To: From: Sami Tolvanen Date: Fri, 5 Mar 2021 08:55:42 -0800 Message-ID: Subject: Re: [PATCH] KVM: arm64: Disable LTO in hyp To: Ard Biesheuvel Cc: Marc Zyngier , Fangrui Song , Nick Desaulniers , James Morse , Nathan Chancellor , Kees Cook , Julien Thierry , Suzuki K Poulose , Catalin Marinas , Will Deacon , linux-arm-kernel , kvmarm , LKML X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210305_165556_808579_67B8DBA1 X-CRM114-Status: GOOD ( 41.43 ) 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, 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. > > // 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 > > > > which is completely untested. I wouldn't be surprised if there were > > more of these somewhere. > > > > A quick grep gives me > > $ objdump -r vmlinux.o |grep BR19 > 000000000005b6e0 R_AARCH64_CONDBR19 hyp_panic > 0000000000418e08 R_AARCH64_CONDBR19 __memcpy > 0000000000418e14 R_AARCH64_CONDBR19 __memcpy These two are in __memmove. This shouldn't be an issue, at least with LTO. > 0000000000003818 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003898 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003918 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003998 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003a18 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003a98 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003b18 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003b98 R_AARCH64_CONDBR19 __kvm_nvhe___guest_exit_panic > 0000000000003c10 R_AARCH64_CONDBR19 __kvm_nvhe___host_exit > 0000000000003c1c R_AARCH64_CONDBR19 __kvm_nvhe___host_exit > 00000000000064f0 R_AARCH64_CONDBR19 __kvm_nvhe_hyp_panic > 000000000000078c R_AARCH64_CONDBR19 __kvm_nvhe___kvm_handle_stub_hvc It looks like all the nvhe code is linked together into kvm_nvhe.o, so I don't think these will cause problems either. Sami _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel