From: Sheng Yang <sheng@linux.intel.com>
To: Avi Kivity <avi@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: VMX: Fix 32bit Windows blue screen with EPT
Date: Thu, 30 Dec 2010 17:05:28 +0800 [thread overview]
Message-ID: <201012301705.28771.sheng@linux.intel.com> (raw)
In-Reply-To: <4D1C4970.9000409@redhat.com>
On Thursday 30 December 2010 16:57:20 Avi Kivity wrote:
> On 12/30/2010 10:35 AM, Sheng Yang wrote:
> > After CR0 is changed during VMExit, the result of kvm_read_cr3() may be
> > different. Commit d95bfcdd7cda4dfdac9588e684bc7c75794a075e "KVM: Fetch
> > guest cr3 from hardware on demand" caused 32bit Windows guest blue
> > screen when using with EPT. This patch fixes it by decache CR3 before
> > CR0 change, for both paging to nonpaging, and nonpaging to paging
> > switch.
> >
> > Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> > ---
> >
> > But I haven't found the exactly point affected by this, any clue?
>
> Can't see it either.
>
> > @@ -1921,8 +1921,7 @@ static void ept_update_paging_mode_cr0(unsigned
> > long *hw_cr0,
> >
> > unsigned long cr0,
> > struct kvm_vcpu *vcpu)
> >
> > {
> >
> > - ulong cr3;
> > -
> > + kvm_read_cr3(vcpu);
>
> Without this line, it fails?
Yes, seems something happened on paging to nonpaging switch process.
>
> I think it's better to call vmx_decache_cr3() explicitly, since it
> explains what we're doing. vmx_decache_cr3 depends on arch.cr0, and
> we're changing that here.
OK.
>
> > if (!(cr0& X86_CR0_PG)) {
> >
> > /* From paging/starting to nonpaging */
> > vmcs_write32(CPU_BASED_VM_EXEC_CONTROL,
> >
> > @@ -1937,11 +1936,8 @@ static void ept_update_paging_mode_cr0(unsigned
> > long *hw_cr0,
> >
> > vmcs_read32(CPU_BASED_VM_EXEC_CONTROL)&
> >
> > ~(CPU_BASED_CR3_LOAD_EXITING |
> >
> > CPU_BASED_CR3_STORE_EXITING));
> >
> > - /* Must fetch cr3 before updating cr0 */
> > - cr3 = kvm_read_cr3(vcpu);
> >
> > vcpu->arch.cr0 = cr0;
> > vmx_set_cr4(vcpu, kvm_read_cr4(vcpu));
> >
> > - vmx_set_cr3(vcpu, cr3);
>
> This is indeed bogus. But what ensures that we'll have the correct
> GUEST_CR3 after enabling paging?
In fact I don't understand why we need this line. All modification is for CR3
reading, why we need to set hardware CR3 again? It should be the same as when we
don't have CR3 accessor I think.
--
regards
Yang, Sheng
>
> > }
> >
> > if (!(cr0& X86_CR0_WP))
next prev parent reply other threads:[~2010-12-30 9:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-30 8:35 [PATCH] KVM: VMX: Fix 32bit Windows blue screen with EPT Sheng Yang
2010-12-30 8:57 ` Avi Kivity
2010-12-30 9:05 ` Sheng Yang [this message]
2010-12-30 9:14 ` Avi Kivity
2010-12-30 9:20 ` Sheng Yang
2010-12-30 9:23 ` Avi Kivity
2010-12-30 9:09 ` Sheng Yang
2010-12-30 9:16 ` Avi Kivity
2010-12-30 9:21 ` [PATCH v2] " Sheng Yang
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=201012301705.28771.sheng@linux.intel.com \
--to=sheng@linux.intel.com \
--cc=avi@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=mtosatti@redhat.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox