Skip to contentSkip to conversation

KVM: x86: move vapic page handling out of fast path

DISCUSSION Marcelo Tosatti 2 participants 2 messages Jun 23, 2008 – Jun 29, 2008

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Marcelo Tosatti

author
#0 lore raw
Quoted 2 lines · Avi Kivity

The page can't be swapped out since its reference count is elevated indefinitely.

AFAICT the page can be swapped out when the guest has exited to userspace and the only reference is the qemu userspace mapping.

Quoted 17 lines
Quoted 2 lines

So, what do you have against this patch ?

We need to move away from reference counts, they make kvm brittle. The patch improves the current state of things (since pages are pinned indefinitely anyway now) but takes the wrong direction for the future.

Note that kvmclock has the same issue, so we might as well share the solution:

struct kvm_fast_guest_page {
    gfn_t gfn;
    struct page *page;
    spinlock_t lock;
    struct list_head link;
}
Quoted 3 lines

The mmu notifier callbacks can scan this list and null any pages that match the gfn being cleared, but in the normal cast, ->page can be accessed directly.

OK, can you please apply this at least:

KVM: move slots_lock acquision down to vapic_exit

There is no need to grab slots_lock if the vapic_page will not be touched.

Signed-off-by Marcelo Tosatti <[email protected]>
arch/x86/kvm/x86.c +2 -2
index 26b051b..29e8983 100644
@@ -2759,8 +2759,10 @@ static void vapic_exit(struct kvm_vcpu *vcpu)
 	if (!apic || !apic->vapic_addr)
 		return;
 
+	down_read(&vcpu->kvm->slots_lock);
 	kvm_release_page_dirty(apic->vapic_page);
 	mark_page_dirty(vcpu->kvm, apic->vapic_addr >> PAGE_SHIFT);
+	up_read(&vcpu->kvm->slots_lock);
 }
 
 static int __vcpu_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)

@@ -2916,9 +2918,7 @@ out:
 
 	post_kvm_run_save(vcpu, kvm_run);
 
-	down_read(&vcpu->kvm->slots_lock);
 	vapic_exit(vcpu);
-	up_read(&vcpu->kvm->slots_lock);
 
 	return r;
 }
#1 lore raw
Quoted 9 lines · Marcelo Tosatti
Quoted 2 lines · Avi Kivity

The page can't be swapped out since its reference count is elevated indefinitely.

AFAICT the page can be swapped out when the guest has exited to userspace and the only reference is the qemu userspace mapping.

That's true, but immaterial, as guests spend most of their time running guest code or sleeping in the kernel.

We could drop the page while the guest is sleeping, but I'd like things to work well even if the guest never idles.

Quoted 8 lines

OK, can you please apply this at least:

KVM: move slots_lock acquision down to vapic_exit

There is no need to grab slots_lock if the vapic_page will not be touched.

Sure, applied.

Signature · 1 line
error compiling committee.c: too many arguments to function