From: Sean Christopherson <seanjc@google.com>
To: Maxim Levitsky <mlevitsk@redhat.com>
Cc: kvm@vger.kernel.org, Jim Mattson <jmattson@google.com>,
Paolo Bonzini <pbonzini@redhat.com>,
"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<x86@kernel.org>, Ingo Molnar <mingo@redhat.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<linux-kernel@vger.kernel.org>, Joerg Roedel <joro@8bytes.org>,
Wanpeng Li <wanpengli@tencent.com>,
"H. Peter Anvin" <hpa@zytor.com>, Borislav Petkov <bp@alien8.de>
Subject: Re: [PATCH v2 6/6] KVM: nVMX: re-evaluate emulation_required on nested VM exit
Date: Tue, 21 Sep 2021 22:55:26 +0000 [thread overview]
Message-ID: <YUpi3rpTsoQ/dzp6@google.com> (raw)
In-Reply-To: <20210830125539.1768833-7-mlevitsk@redhat.com>
On Mon, Aug 30, 2021, Maxim Levitsky wrote:
> If L1 had invalid state on VM entry (can happen on SMM transactions
> when we enter from real mode, straight to nested guest),
>
> then after we load 'host' state from VMCS12, the state has to become
> valid again, but since we load the segment registers with
> __vmx_set_segment we weren't always updating emulation_required.
Because I'm an idiot.
> Update emulation_required explicitly at end of load_vmcs12_host_state.
Can you also add
Reported-by: kernel test robot <oliver.sang@intel.com>
which is how I ended up here, sort of.
Fixes: 816be9e9be8d ("KVM: nVMX: Don't evaluate "emulation required" on nested VM-Exit")
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
The below is overkill. The state after VM-Exit _can't_ be invalid, which was the
whole point of moving to __vmx_set_segment(), I just forgot the minor detail of
clearing emulation_required. So just:
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index bc6327950657..55ac7211fb37 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -4314,6 +4314,12 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
if (nested_vmx_load_msr(vcpu, vmcs12->vm_exit_msr_load_addr,
vmcs12->vm_exit_msr_load_count))
nested_vmx_abort(vcpu, VMX_ABORT_LOAD_HOST_MSR_FAIL);
+
+ /*
+ * All relevant vmcs12 host state is checked prior to VM-Entry, thus
+ * L1 guest can never be invalid after VM-Exit.
+ */
+ to_vmx(vcpu)->emulation_required = false;
}
static inline u64 nested_vmx_get_vmcs01_guest_efer(struct vcpu_vmx *vmx)
> arch/x86/kvm/vmx/nested.c | 2 ++
> arch/x86/kvm/vmx/vmx.c | 8 ++++----
> arch/x86/kvm/vmx/vmx.h | 1 +
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 1a05ae83dae5..f915e1ac589c 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -4319,6 +4319,8 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
> if (nested_vmx_load_msr(vcpu, vmcs12->vm_exit_msr_load_addr,
> vmcs12->vm_exit_msr_load_count))
> nested_vmx_abort(vcpu, VMX_ABORT_LOAD_HOST_MSR_FAIL);
> +
> + to_vmx(vcpu)->emulation_required = vmx_emulation_required(vcpu);
> }
>
> static inline u64 nested_vmx_get_vmcs01_guest_efer(struct vcpu_vmx *vmx)
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 02d061f5956a..7ff1e1daeb0a 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1323,7 +1323,7 @@ static void vmx_vcpu_put(struct kvm_vcpu *vcpu)
> vmx_prepare_switch_to_host(to_vmx(vcpu));
> }
>
> -static bool emulation_required(struct kvm_vcpu *vcpu)
> +bool vmx_emulation_required(struct kvm_vcpu *vcpu)
> {
> return emulate_invalid_guest_state && !vmx_guest_state_valid(vcpu);
> }
> @@ -1367,7 +1367,7 @@ void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags)
> vmcs_writel(GUEST_RFLAGS, rflags);
>
> if ((old_rflags ^ vmx->rflags) & X86_EFLAGS_VM)
> - vmx->emulation_required = emulation_required(vcpu);
> + vmx->emulation_required = vmx_emulation_required(vcpu);
> }
>
> u32 vmx_get_interrupt_shadow(struct kvm_vcpu *vcpu)
> @@ -3077,7 +3077,7 @@ void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
> }
>
> /* depends on vcpu->arch.cr0 to be set to a new value */
> - vmx->emulation_required = emulation_required(vcpu);
> + vmx->emulation_required = vmx_emulation_required(vcpu);
> }
>
> static int vmx_get_max_tdp_level(void)
> @@ -3330,7 +3330,7 @@ static void vmx_set_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int
> {
> __vmx_set_segment(vcpu, var, seg);
>
> - to_vmx(vcpu)->emulation_required = emulation_required(vcpu);
> + to_vmx(vcpu)->emulation_required = vmx_emulation_required(vcpu);
> }
>
> static void vmx_get_cs_db_l_bits(struct kvm_vcpu *vcpu, int *db, int *l)
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index 4858c5fd95f2..3a587c51a8d1 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -359,6 +359,7 @@ void vmx_prepare_switch_to_guest(struct kvm_vcpu *vcpu);
> void vmx_set_host_fs_gs(struct vmcs_host_state *host, u16 fs_sel, u16 gs_sel,
> unsigned long fs_base, unsigned long gs_base);
> int vmx_get_cpl(struct kvm_vcpu *vcpu);
> +bool vmx_emulation_required(struct kvm_vcpu *vcpu);
> unsigned long vmx_get_rflags(struct kvm_vcpu *vcpu);
> void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags);
> u32 vmx_get_interrupt_shadow(struct kvm_vcpu *vcpu);
> --
> 2.26.3
>
prev parent reply other threads:[~2021-09-21 22:55 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-30 12:55 [PATCH v2 0/6] KVM: few more SMM fixes Maxim Levitsky
2021-08-30 12:55 ` [PATCH v2 1/6] KVM: SVM: restore the L1 host state prior to resuming nested guest on SMM exit Maxim Levitsky
2021-08-30 12:55 ` [PATCH v2 2/6] KVM: x86: force PDPTR reload " Maxim Levitsky
2021-08-30 12:55 ` [PATCH v2 3/6] KVM: nSVM: call KVM_REQ_GET_NESTED_STATE_PAGES on exit from SMM mode Maxim Levitsky
2021-08-30 12:55 ` [PATCH v2 4/6] KVM: VMX: synthesize invalid VM exit when emulating invalid guest state Maxim Levitsky
2021-08-30 12:55 ` [PATCH v2 5/6] KVM: nVMX: don't fail nested VM entry on invalid guest state if !from_vmentry Maxim Levitsky
2021-10-08 23:37 ` Sean Christopherson
2021-08-30 12:55 ` [PATCH v2 6/6] KVM: nVMX: re-evaluate emulation_required on nested VM exit Maxim Levitsky
2021-09-21 22:55 ` Sean Christopherson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YUpi3rpTsoQ/dzp6@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=mlevitsk@redhat.com \
--cc=pbonzini@redhat.com \
--cc=tglx@linutronix.de \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.