All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Radim Krčmář" <rkrcmar@redhat.com>
To: Bruce Rogers <brogers@suse.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	namit@cs.technion.ac.il, stable@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: fix ordering of cr0 initialization code in vmx_cpu_reset
Date: Thu, 28 Apr 2016 21:08:06 +0200	[thread overview]
Message-ID: <20160428190805.GA9655@potion> (raw)
In-Reply-To: <1461351363-28094-1-git-send-email-brogers@suse.com>

2016-04-22 12:56-0600, Bruce Rogers:
> Commit d28bc9dd25ce reversed the order of two lines which initialize cr0,
> allowing the current (old) cr0 value to mess up vcpu initialization.
> This was observed in the checks for cr0 X86_CR0_WP bit in the context of
> kvm_mmu_reset_context(). Besides, setting vcpu->arch.cr0 after vmx_set_cr0()
> is completely redundant. Change the order back to ensure proper vcpu
> initialization.
> 
> The combination of booting with ovmf firmware when guest vcpus > 1 and kvm's
> ept=N option being set results in a VM-entry failure. This patch fixes that.

Greg pointed out missing,
  Cc: stable@vger.kernel.org
when stable@vger.kernel.org was Cc'd. Adding
  Fixes: d28bc9dd25ce ("KVM: x86: INIT and reset sequences are different")
would be nice too (even when it is redundant).

> Signed-off-by: Bruce Rogers <brogers@suse.com>
> ---
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> @@ -5046,8 +5046,8 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
>  	cr0 = X86_CR0_NW | X86_CR0_CD | X86_CR0_ET;
> -	vmx_set_cr0(vcpu, cr0); /* enter rmode */
>  	vmx->vcpu.arch.cr0 = cr0;
> +	vmx_set_cr0(vcpu, cr0); /* enter rmode */

So vmx_set_cr0() has a code that depends on vmx->vcpu.arch.cr0 being
already set the to new value.  Do you know what function is it?

I think we better set vmx->vcpu.arch.cr0 early in vmx_set_cr0().
Or do other callsites somehow depend on the old cr0 value?

Thanks.

  parent reply	other threads:[~2016-04-28 19:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-22 18:56 [PATCH] KVM: x86: fix ordering of cr0 initialization code in vmx_cpu_reset Bruce Rogers
2016-04-22 22:18 ` Greg KH
2016-04-28 19:08 ` Radim Krčmář [this message]
2016-04-28 20:32   ` Bruce Rogers
2016-04-28 20:32     ` Bruce Rogers

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=20160428190805.GA9655@potion \
    --to=rkrcmar@redhat.com \
    --cc=brogers@suse.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namit@cs.technion.ac.il \
    --cc=stable@vger.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.