Re: [PATCH 7/8] KVM: MMU: VMX cr3 cache support

[Marcelo Tosatti <marcelo@kvack.org>]

Hi Zhao,

On Thu, Mar 06, 2008 at 09:15:59PM +0800, Zhao Forrest wrote:
> > @@ -1219,53 +1229,75 @@ static void nonpaging_prefetch_page(struct kvm_vcpu *vcpu,
> >
> >  static void mmu_free_roots(struct kvm_vcpu *vcpu)
> >  {
> > -       int i;
> > +       int i, j;
> >        struct kvm_mmu_page *sp;
> >
> > -       if (!VALID_PAGE(vcpu->arch.mmu.root_hpa))
> > -               return;
> > +       /*
> > +        * Skip to the next cr3 filter entry and free it (if it's occupied).
> > +        */
> > +       vcpu->arch.cr3_cache_idx++;
> > +       if (unlikely(vcpu->arch.cr3_cache_idx >= vcpu->arch.cr3_cache_limit))
> > +               vcpu->arch.cr3_cache_idx = 0;
> > +
> > +       j = vcpu->arch.cr3_cache_idx;
> > +       /*
> > +        * Clear the guest-visible entry.
> > +        */
> > +       if (vcpu->arch.cr3_cache) {
> > +               vcpu->arch.cr3_cache->entry[j].guest_cr3 = 0;
> > +               vcpu->arch.cr3_cache->entry[j].host_cr3 = 0;
> > +       }
> Here CR3_TARGET_VALUEx is not written.
> ......
> > @@ -1322,10 +1353,23 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
> >
> >  static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
> >  {
> > +       struct kvm_cr3_cache *cache;
> > +       int idx;
> > +
> >        vmx_flush_tlb(vcpu);
> >        vmcs_writel(GUEST_CR3, cr3);
> >        if (vcpu->arch.cr0 & X86_CR0_PE)
> >                vmx_fpu_deactivate(vcpu);
> > +
> > +       if (!vcpu->arch.cr3_cache)
> > +               return;
> > +
> > +       idx = vcpu->arch.cr3_cache_idx;
> > +       cache = vcpu->arch.cr3_cache;
> > +
> > +       cache->entry[idx].host_cr3 = cr3;
> > +       cache->entry[idx].guest_cr3 = vcpu->arch.cr3;
> > +       vmcs_writel(CR3_TARGET_VALUE0 + idx*2, cr3);
> >  }
> >
> Here CR3_TARGET_VALUEx is written.
> My question is:
> 1 why is vmcs_writel(CR3_TARGET_VALUE0 + idx*2, cr3); called by
> vmx_set_cr3(), but not called by mmu_free_roots()?

By clearing guest_cr3 entry of the shared area we avoid the guest from
using it.

So its unecessary to also clear the corresponding CR3_TARGET_VALUE0
register.

> 2 since "cache" is also mapped to guest OS. Is calling
> vmcs_writel(CR3_TARGET_VALUE0 + idx*2, cr3); necessary?

As said above, no, because the guest will check
cache->entry[idx].guest_cr3 before attempting to use a cached host_cr3
value.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/