Message ID | 20230707084328.2563454-1-suhui@nfschina.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f45:0:b0:3ea:f831:8777 with SMTP id v5csp3118458vqx; Fri, 7 Jul 2023 01:52:50 -0700 (PDT) X-Google-Smtp-Source: APBJJlHZy0wlY05Vt0rDVnoqzKuPE/HSG2AU3v2YvYJmeimPVP7hO6sPKHiWZcGEgGwZ+3XGzEwR X-Received: by 2002:a05:6a00:1354:b0:66a:5466:25bd with SMTP id k20-20020a056a00135400b0066a546625bdmr4863787pfu.15.1688719969730; Fri, 07 Jul 2023 01:52:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1688719969; cv=none; d=google.com; s=arc-20160816; b=GIXXFnp0cCOyyZZ2lCqmMs9clTe0+SiQBWVbWPbEfhVeEBcp8h61GdLN+qKDt6mWAh zipvw8yk8dmIPwFlIVNUBQ1EnSLZbIigs7EoSqcMMzbqSeVUuQcg6KtnWQzomxdjZoZP CN4ZNBc1xQj6cNeaFHQf3fVT8T9IltSjSq7YInHD0JQh/UzhfzSPDJL6sc50OyD2CAoX HQ8Oc65Hw6wFCuBX57gDJ2kRxfvKd/vzE0f7Bd36K2TycWeVisSwfBJzB1uJLGNovWgD GQ31TppfDIDJVEEBofujAOuZVcLFa400eqB9KN0IFWyL53sK3OnYL9S+f2YPKqDjyMbl DO8g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=qx5Wgf3Rnub5/BSTYIWNYnpxva7xacNTi0DR552B6NI=; fh=oUc/7Lw3E4XzrclvZB1FUKExhvKqWDzt9V56j/ALygo=; b=gUCqP4id5tk7jgSo8rLJDqwWhBJlcACzm638L8Wh6zvb0ahDR4D4tzMDcnyk/viZ6A G53eDLf206nILID2eVfIZDiCI/XBKSVhwxpVAQ+rm1XPLT3ODClmeTEXY/6bxU0piXZi 88xQzVQ+aQvLZ/bo031C6hrfge/8YeSoLTQDpxL5DEJ7trjJX0xBUwTPBX0i8Ch4gL9I bI3otQFIMbAYCKm7l6iZvPmy+8w1gVZqP/xqPy6GRErsm2CWBeaTLLmJImFK2J3bwbDU Z4UQc7DxWNwx0G5nehcgKY/DH37prtaOtHkvyW6LfhcTn+uPg1OWnoxJxR6IBAioXhil Ichg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id bq24-20020a056a000e1800b0068188beefedsi3376600pfb.88.2023.07.07.01.52.35; Fri, 07 Jul 2023 01:52:49 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231802AbjGGIoN (ORCPT <rfc822;hadasmailinglist@gmail.com> + 99 others); Fri, 7 Jul 2023 04:44:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34634 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229642AbjGGIoM (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 7 Jul 2023 04:44:12 -0400 Received: from mail.nfschina.com (unknown [42.101.60.195]) by lindbergh.monkeyblade.net (Postfix) with SMTP id 479E31FD8; Fri, 7 Jul 2023 01:44:11 -0700 (PDT) Received: from localhost.localdomain (unknown [180.167.10.98]) by mail.nfschina.com (Maildata Gateway V2.8.8) with ESMTPA id 71567603362EB; Fri, 7 Jul 2023 16:43:50 +0800 (CST) X-MD-Sfrom: suhui@nfschina.com X-MD-SrcIP: 180.167.10.98 From: Su Hui <suhui@nfschina.com> To: seanjc@google.com, pbonzini@redhat.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org, Su Hui <suhui@nfschina.com> Subject: [PATCH] KVM: VMX: Avoid noinstr warning Date: Fri, 7 Jul 2023 16:43:28 +0800 Message-Id: <20230707084328.2563454-1-suhui@nfschina.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,RDNS_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1770751231008539686?= X-GMAIL-MSGID: =?utf-8?q?1770751231008539686?= |
Series |
KVM: VMX: Avoid noinstr warning
|
|
Commit Message
Su Hui
July 7, 2023, 8:43 a.m. UTC
vmlinux.o: warning: objtool: vmx_vcpu_enter_exit+0x2d8:
call to vmread_error_trampoline() leaves .noinstr.text section
Signed-off-by: Su Hui <suhui@nfschina.com>
---
arch/x86/kvm/vmx/vmx_ops.h | 2 ++
1 file changed, 2 insertions(+)
Comments
On Fri, Jul 07, 2023, Su Hui wrote: > vmlinux.o: warning: objtool: vmx_vcpu_enter_exit+0x2d8: > call to vmread_error_trampoline() leaves .noinstr.text section > > Signed-off-by: Su Hui <suhui@nfschina.com> > --- > arch/x86/kvm/vmx/vmx_ops.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/x86/kvm/vmx/vmx_ops.h b/arch/x86/kvm/vmx/vmx_ops.h > index ce47dc265f89..54f86ce2ad60 100644 > --- a/arch/x86/kvm/vmx/vmx_ops.h > +++ b/arch/x86/kvm/vmx/vmx_ops.h > @@ -112,6 +112,7 @@ static __always_inline unsigned long __vmcs_readl(unsigned long field) > > #else /* !CONFIG_CC_HAS_ASM_GOTO_OUTPUT */ > > + instrumentation_begin(); > asm volatile("1: vmread %2, %1\n\t" > ".byte 0x3e\n\t" /* branch taken hint */ > "ja 3f\n\t" > @@ -139,6 +140,7 @@ static __always_inline unsigned long __vmcs_readl(unsigned long field) > _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_ONE_REG, %1) > > : ASM_CALL_CONSTRAINT, "=&r"(value) : "r"(field) : "cc"); > + instrumentation_end(); Tagging the entire thing as instrumentable is not correct, e.g. instrumentation isn't magically safe when doing VMREAD immediately after VM-Exit. Enabling instrumentation for VM-Fail paths isn't exactly safe either, but odds are very good that the system has major issue if a VMX instruction (other than VMLAUNCH/VMRESUME) gets VM-Fail, in which case logging the error takes priority. Compile tested only, but I think the below is the least awful solution. That will also allow the CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y case to use vmread_error() instead of open coding an equivalent (hence the "PATCH 1/2"). I'll post patches after testing. Thanks! From: Sean Christopherson <seanjc@google.com> Date: Thu, 13 Jul 2023 13:45:35 -0700 Subject: [PATCH 1/2] KVM: VMX: Make VMREAD error path play nice with noinstr Mark vmread_error_trampoline() as noinstr, and add a second trampoline for the CONFIG_CC_HAS_ASM_GOTO_OUTPUT=n case to enable instrumentation when handling VM-Fail on VMREAD. VMREAD is used in various noinstr flows, e.g. immediately after VM-Exit, and objtool rightly complains that the call to the error trampoline leaves a no-instrumentation section without annotating that it's safe to do so. vmlinux.o: warning: objtool: vmx_vcpu_enter_exit+0xc9: call to vmread_error_trampoline() leaves .noinstr.text section Note, strictly speaking, enabling instrumentation in the VM-Fail path isn't exactly safe, but if VMREAD fails the kernel/system is likely hosed anyways, and logging that there is a fatal error is more important than *maybe* encountering slightly unsafe instrumentation. Reported-by: Su Hui <suhui@nfschina.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/vmx/vmenter.S | 8 ++++---- arch/x86/kvm/vmx/vmx.c | 18 ++++++++++++++---- arch/x86/kvm/vmx/vmx_ops.h | 9 ++++++++- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S index 07e927d4d099..be275a0410a8 100644 --- a/arch/x86/kvm/vmx/vmenter.S +++ b/arch/x86/kvm/vmx/vmenter.S @@ -303,10 +303,8 @@ SYM_FUNC_START(vmx_do_nmi_irqoff) VMX_DO_EVENT_IRQOFF call asm_exc_nmi_kvm_vmx SYM_FUNC_END(vmx_do_nmi_irqoff) - -.section .text, "ax" - #ifndef CONFIG_CC_HAS_ASM_GOTO_OUTPUT + /** * vmread_error_trampoline - Trampoline from inline asm to vmread_error() * @field: VMCS field encoding that failed @@ -335,7 +333,7 @@ SYM_FUNC_START(vmread_error_trampoline) mov 3*WORD_SIZE(%_ASM_BP), %_ASM_ARG2 mov 2*WORD_SIZE(%_ASM_BP), %_ASM_ARG1 - call vmread_error + call vmread_error_trampoline2 /* Zero out @fault, which will be popped into the result register. */ _ASM_MOV $0, 3*WORD_SIZE(%_ASM_BP) @@ -357,6 +355,8 @@ SYM_FUNC_START(vmread_error_trampoline) SYM_FUNC_END(vmread_error_trampoline) #endif +.section .text, "ax" + SYM_FUNC_START(vmx_do_interrupt_irqoff) VMX_DO_EVENT_IRQOFF CALL_NOSPEC _ASM_ARG1 SYM_FUNC_END(vmx_do_interrupt_irqoff) diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 0ecf4be2c6af..d7cf35edda1b 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -441,13 +441,23 @@ do { \ pr_warn_ratelimited(fmt); \ } while (0) -void vmread_error(unsigned long field, bool fault) +noinline void vmread_error(unsigned long field) { - if (fault) + vmx_insn_failed("vmread failed: field=%lx\n", field); +} + +#ifndef CONFIG_CC_HAS_ASM_GOTO_OUTPUT +noinstr void vmread_error_trampoline2(unsigned long field, bool fault) +{ + if (fault) { kvm_spurious_fault(); - else - vmx_insn_failed("vmread failed: field=%lx\n", field); + } else { + instrumentation_begin(); + vmread_error(field); + instrumentation_end(); + } } +#endif noinline void vmwrite_error(unsigned long field, unsigned long value) { diff --git a/arch/x86/kvm/vmx/vmx_ops.h b/arch/x86/kvm/vmx/vmx_ops.h index ce47dc265f89..5fa74779a37a 100644 --- a/arch/x86/kvm/vmx/vmx_ops.h +++ b/arch/x86/kvm/vmx/vmx_ops.h @@ -10,7 +10,7 @@ #include "vmcs.h" #include "../x86.h" -void vmread_error(unsigned long field, bool fault); +void vmread_error(unsigned long field); void vmwrite_error(unsigned long field, unsigned long value); void vmclear_error(struct vmcs *vmcs, u64 phys_addr); void vmptrld_error(struct vmcs *vmcs, u64 phys_addr); @@ -31,6 +31,13 @@ void invept_error(unsigned long ext, u64 eptp, gpa_t gpa); * void vmread_error_trampoline(unsigned long field, bool fault); */ extern unsigned long vmread_error_trampoline; + +/* + * The second VMREAD error trampoline, called from the assembly trampoline, + * exists primarily to enable instrumentation for the VM-Fail path. + */ +void vmread_error_trampoline2(unsigned long field, bool fault); + #endif static __always_inline void vmcs_check16(unsigned long field) base-commit: 255006adb3da71bb75c334453786df781b415f54 --
diff --git a/arch/x86/kvm/vmx/vmx_ops.h b/arch/x86/kvm/vmx/vmx_ops.h index ce47dc265f89..54f86ce2ad60 100644 --- a/arch/x86/kvm/vmx/vmx_ops.h +++ b/arch/x86/kvm/vmx/vmx_ops.h @@ -112,6 +112,7 @@ static __always_inline unsigned long __vmcs_readl(unsigned long field) #else /* !CONFIG_CC_HAS_ASM_GOTO_OUTPUT */ + instrumentation_begin(); asm volatile("1: vmread %2, %1\n\t" ".byte 0x3e\n\t" /* branch taken hint */ "ja 3f\n\t" @@ -139,6 +140,7 @@ static __always_inline unsigned long __vmcs_readl(unsigned long field) _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_ONE_REG, %1) : ASM_CALL_CONSTRAINT, "=&r"(value) : "r"(field) : "cc"); + instrumentation_end(); return value; #endif /* CONFIG_CC_HAS_ASM_GOTO_OUTPUT */