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=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT 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 820DCC10F11 for ; Wed, 10 Apr 2019 16:08:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5470E20652 for ; Wed, 10 Apr 2019 16:08:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731415AbfDJQIa (ORCPT ); Wed, 10 Apr 2019 12:08:30 -0400 Received: from mga11.intel.com ([192.55.52.93]:26117 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731520AbfDJQI3 (ORCPT ); Wed, 10 Apr 2019 12:08:29 -0400 X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Apr 2019 09:08:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,332,1549958400"; d="scan'208,223";a="159997264" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.181]) by fmsmga002.fm.intel.com with ESMTP; 10 Apr 2019 09:08:25 -0700 Date: Wed, 10 Apr 2019 09:08:25 -0700 From: Sean Christopherson To: Paolo Bonzini Cc: Krish Sadhukhan , kvm@vger.kernel.org, rkrcmar@redhat.com, jmattson@google.com Subject: Re: [PATCH 5/6 v5][KVM nVMX]: nested_vmx_check_vmentry_postreqs() should return VMX_EXIT_REASONS_FAILED_VMENTRY | EXIT_REASON_INVALID_STATE for error conditions Message-ID: <20190410160825.GA26808@linux.intel.com> References: <20190408213516.17966-1-krish.sadhukhan@oracle.com> <20190408213516.17966-6-krish.sadhukhan@oracle.com> <15d3f7f7-f377-1295-55a7-5651d7d44349@redhat.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="tKW2IUtsqtDRztdT" Content-Disposition: inline In-Reply-To: <15d3f7f7-f377-1295-55a7-5651d7d44349@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org --tKW2IUtsqtDRztdT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Apr 10, 2019 at 11:50:50AM +0200, Paolo Bonzini wrote: > On 08/04/19 23:35, Krish Sadhukhan wrote: > > ..to reflect the architectural Exit Reason for VM-entry failures due to > > invalid guest state. > > > > Signed-off-by: Krish Sadhukhan > > Suggested-by: Sean Christopherson > > --- > > arch/x86/kvm/vmx/nested.c | 15 +++++++++++---- > > 1 file changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > index 1ec5ddc4ea50..bde17d079a36 100644 > > --- a/arch/x86/kvm/vmx/nested.c > > +++ b/arch/x86/kvm/vmx/nested.c > > @@ -2701,11 +2701,14 @@ static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu, > > *exit_qual = ENTRY_FAIL_DEFAULT; > > > > if (nested_check_guest_cregs_dregs_msrs(vcpu, vmcs12)) > > - return 1; > > + return VMX_EXIT_REASONS_FAILED_VMENTRY | > > + EXIT_REASON_INVALID_STATE; > > > > if (nested_vmx_check_vmcs_link_ptr(vcpu, vmcs12)) { > > *exit_qual = ENTRY_FAIL_VMCS_LINK_PTR; > > - return 1; > > + > > + return VMX_EXIT_REASONS_FAILED_VMENTRY | > > + EXIT_REASON_INVALID_STATE; > > } > > > > /* > > @@ -2724,13 +2727,17 @@ static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu, > > ia32e != !!(vmcs12->guest_ia32_efer & EFER_LMA) || > > ((vmcs12->guest_cr0 & X86_CR0_PG) && > > ia32e != !!(vmcs12->guest_ia32_efer & EFER_LME))) > > - return 1; > > + > > + return VMX_EXIT_REASONS_FAILED_VMENTRY | > > + EXIT_REASON_INVALID_STATE; > > } > > > > if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS) && > > (is_noncanonical_address(vmcs12->guest_bndcfgs & PAGE_MASK, vcpu) || > > (vmcs12->guest_bndcfgs & MSR_IA32_BNDCFGS_RSVD))) > > - return 1; > > + > > + return VMX_EXIT_REASONS_FAILED_VMENTRY | > > + EXIT_REASON_INVALID_STATE; > > > > return 0; > > } > > > > This gives the reader a false impression that the return value is > actually reflected in the exit reason If anything I would change those > to -EINVAL, similar to what you did in patch 4 (but without applying > patch 3 which, as I understand it, is mostly a "trick" to make this > patch less verbose). Good point, though IMO it'd be better to go one step further and actually consume the return value in nested_vmx_enter_non_root_mode(). For me, having the exit reason in nested_vmx_check_vmentry_postreqs() is a nice mental reminder that "postreqs" is referring to checks that happen once the CPU has "committed" to VM-Enter. What about the attached patch as fixup? --tKW2IUtsqtDRztdT Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-KVM-nVMX-Fixup-nested_vmx_check_vmentry_postreqs-ret.patch" >From 377aa4cc91aa2af140ade76f61c30040a77fd660 Mon Sep 17 00:00:00 2001 From: Sean Christopherson Date: Wed, 10 Apr 2019 09:03:16 -0700 Subject: [PATCH] KVM: nVMX: Fixup nested_vmx_check_vmentry_postreqs() return val handling Signed-off-by: Sean Christopherson --- arch/x86/kvm/vmx/nested.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 6bbb28ef313e..94405dcaccec 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -2691,14 +2691,11 @@ static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu, *exit_qual = ENTRY_FAIL_DEFAULT; if (nested_check_guest_cregs_dregs_msrs(vcpu, vmcs12)) - return VMX_EXIT_REASONS_FAILED_VMENTRY | - EXIT_REASON_INVALID_STATE; + return EXIT_REASON_INVALID_STATE; if (nested_vmx_check_vmcs_link_ptr(vcpu, vmcs12)) { *exit_qual = ENTRY_FAIL_VMCS_LINK_PTR; - - return VMX_EXIT_REASONS_FAILED_VMENTRY | - EXIT_REASON_INVALID_STATE; + return EXIT_REASON_INVALID_STATE; } /* @@ -2717,17 +2714,13 @@ static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu, ia32e != !!(vmcs12->guest_ia32_efer & EFER_LMA) || ((vmcs12->guest_cr0 & X86_CR0_PG) && ia32e != !!(vmcs12->guest_ia32_efer & EFER_LME))) - - return VMX_EXIT_REASONS_FAILED_VMENTRY | - EXIT_REASON_INVALID_STATE; + return EXIT_REASON_INVALID_STATE; } if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS) && (is_noncanonical_address(vmcs12->guest_bndcfgs & PAGE_MASK, vcpu) || (vmcs12->guest_bndcfgs & MSR_IA32_BNDCFGS_RSVD))) - - return VMX_EXIT_REASONS_FAILED_VMENTRY | - EXIT_REASON_INVALID_STATE; + return EXIT_REASON_INVALID_STATE; return 0; } @@ -2975,7 +2968,7 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry) struct vcpu_vmx *vmx = to_vmx(vcpu); struct vmcs12 *vmcs12 = get_vmcs12(vcpu); bool evaluate_pending_interrupts; - u32 exit_reason = EXIT_REASON_INVALID_STATE; + u32 exit_reason; u32 exit_qual; evaluate_pending_interrupts = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL) & @@ -3001,7 +2994,9 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry) return -1; } - if (nested_vmx_check_vmentry_postreqs(vcpu, vmcs12, &exit_qual)) + exit_reason = nested_vmx_check_vmentry_postreqs(vcpu, vmcs12, + &exit_qual); + if (exit_reason) goto vmentry_fail_vmexit; } -- 2.21.0 --tKW2IUtsqtDRztdT--