Re: [PATCH 2/3] KVM: VMX: Simplify pdptr and cr3 management

From: Sheng Yang <sheng@linux.intel.com>
To: Avi Kivity <avi@redhat.com>
Cc: kvm@vger.kernel.org, Marcelo Tosatti <mtosatti@redhat.com>,
	Joerg Roedel <joerg.roedel@amd.com>
Subject: Re: [PATCH 2/3] KVM: VMX: Simplify pdptr and cr3 management
Date: Tue, 2 Jun 2009 17:30:37 +0800	[thread overview]
Message-ID: <200906021730.38414.sheng@linux.intel.com> (raw)
In-Reply-To: <4A24F043.6070109@redhat.com>

On Tuesday 02 June 2009 17:26:27 Avi Kivity wrote:
> Sheng Yang wrote:
> > On Monday 01 June 2009 21:22:02 Avi Kivity wrote:
> >> Instead of reading the PDPTRs from memory after every exit (which is
> >> slow and wrong, as the PDPTRs are stored on the cpu), sync the PDPTRs
> >> from memory to the VMCS before entry, and from the VMCS to memory after
> >> exit. Do the same for cr3.
> >
> > Thanks for fixing!
> >
> > After review my original code, I found a potential bug. For SDM 3B have
> > this:
> >
> > 23.3.4 Saving Non-Register State
> > ...
> > If the logical processor supports the 1-setting of the “enable EPT” VM-
> > execution control, values are saved into the four (4) PDPTE fields as
> > follows: — If the “enable EPT” VM-execution control is 1 and the logical
> > processor was using PAE paging at the time of the VM exit, the PDPTE
> > values currently in use are saved:
> > • The values saved into bits 11:9 of each of the fields is undefined.
> > • If the value saved into one of the fields has bit 0 (present) clear,
> > the value saved into bits 63:1 of that field is undefined. That value
> > need not correspond to the value that was loaded by VM entry or to any
> > value that might have been loaded in VMX non-root operation.
> > • If the value saved into one of the fields has bit 0 (present) set, the
> > value saved into bits 63:12 of the field is a guest-physical address.
> > — If the “enable EPT” VM-execution control is 0 or the logical processor
> > was not using PAE paging at the time of the VM exit, the values saved are
> > undefined.
> >
> > But drop the ept_load_pdptrs() when exit and add it in cr0 handling
> > result in Windows PAE guest hang on boot. I am checking it now. Any
> > thoughts?...
>
> You mean with the new code?  What version of Windows exactly?
>
> I'll check it out, though EPTs are a little hard to find here.

No, no, not with the new code. For CPU can load pdptrs if EPT enabled with PAE 
from VM exit, there should not be necessary load it explicitly. So I estimate 
the ept_load_pdptr() in exit handler, and put it in CR0 handling. Just tried 
to optimize load-pdptr according to the spec, but not got the desired 
result...

So I am trying to find the failure reason...

-- 
regards
Yang, Sheng