Re: cr3 OOS optimisation breaks 32-bit GNU/kFreeBSD guest

Marcelo Tosatti
2009-04-03T21:46:26Z

On Tue, Mar 24, 2009 at 11:47:33AM +0200, Avi Kivity wrote:
>> index 2ea8262..48169d7 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -3109,6 +3109,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>> kvm_write_guest_time(vcpu);
>> if (test_and_clear_bit(KVM_REQ_MMU_SYNC, &vcpu->requests))
>> kvm_mmu_sync_roots(vcpu);
>> + if (test_and_clear_bit(KVM_REQ_MMU_GLOBAL_SYNC, &vcpu->requests))
>> + kvm_mmu_sync_global(vcpu);
>> if (test_and_clear_bit(KVM_REQ_TLB_FLUSH, &vcpu->requests))
>> kvm_x86_ops->tlb_flush(vcpu);
>> if (test_and_clear_bit(KVM_REQ_REPORT_TPR_ACCESS
>
> Windows will (I think) write a PDE on every context switch, so this
> effectively disables global unsync for that guest.
>
> What about recursively syncing the newly linked page in FNAME(fetch)()?
> If the page isn't global, this becomes a no-op, so no new overhead. The
> only question is the expense when linking a populated top-level page,
> especially in long mode.

How about this?

KVM: MMU: sync global pages on fetch()

If an unsync global page becomes unreachable via the shadow tree, which
can happen if one its parent pages is zapped, invlpg will fail to
invalidate translations for gvas contained in such unreachable pages.

So sync global pages in fetch().

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 09782a9..728be72 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -308,8 +308,14 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
break;
}

- if (is_shadow_present_pte(*sptep) && !is_large_pte(*sptep))
+ if (is_shadow_present_pte(*sptep) && !is_large_pte(*sptep)) {
+ if (level-1 == PT_PAGE_TABLE_LEVEL) {
+ shadow_page = page_header(__pa(sptep));
+ if (shadow_page->unsync && shadow_page->global)
+ kvm_sync_page(vcpu, shadow_page);
+ }
continue;
+ }

if (is_large_pte(*sptep)) {
rmap_remove(vcpu->kvm, sptep);