Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu()

[Avi Kivity <avi@redhat.com>]

On 02/08/2012 05:43 PM, Takuya Yoshikawa wrote:
> [Dropped non-kvm members from cc]
>
> Marcelo Tosatti <mtosatti@redhat.com> wrote:
>
> > VGABIOS mode constantly destroys and creates 0xa0000 slot, so
> > performance is required for KVM_SET_MEM too (it can probably be fixed in
> > qemu, but older qemu's must be supported).
>
> Apart from srcu, I see some problems concerning slot creation/destruction:
> heavy shadow flush (and extra write protection).
>
>
> Look at __kvm_set_memory_region().
>
> 1. When we invalidate a memory slot, we call kvm_arch_flush_shadow() and
> zap all shadow pages, not restricted to that slot.
>
> 	/* From this point no new shadow pages pointing to a deleted
> 	 * memslot will be created.
> 	 *
> 	 * validation of sp->gfn happens in:
> 	 *	- gfn_to_hva (kvm_read_guest, gfn_to_pfn)
> 	 *	- kvm_is_visible_gfn (mmu_check_roots)
> 	 */
> 	kvm_arch_flush_shadow(kvm);

The workloads that exercise slot removal heavily usually do so in a
tight loop, so flushing all shadow is not too bad (not much to flush).

>
> 2. When we create(and shift?) a memory slot, we call kvm_arch_flush_shadow()
> to clear all mmio sptes, again not restricted to that slot.
>
> 	/*
> 	 * If the new memory slot is created, we need to clear all
> 	 * mmio sptes.
> 	 */
> 	if (npages && old.base_gfn != mem->guest_phys_addr >> PAGE_SHIFT)
> 		kvm_arch_flush_shadow(kvm);

This is pretty rare outside the previous scenario (memory/pci hotplug).

>
> 3. In addition to these, we do write-protect all pages in that slot in
> kvm_arch_commit_memory_region() every time.
>
>
> For 1:  I made a patch to restrict the flush to that slot by using sp->slot_bitmap.
> (seems working here)
>
> For 2:  I think we can do the same:  not 100% sure yet because I am still
> struggling with the "mmio spte optimization" code.  (really hacky ...)
>
> For 3:  I think doing both "write protection" and "shadow flush" is unnecessary.

Maybe it only needs to be done if the only change is enabling dirty logging.

> BTW do we really need fast slot creation/destruction?

Not really, but it's good to have infrastructure that copes with
different workloads.  If the patches keep the code simple I think it's a
good thing to have.

-- 
error compiling committee.c: too many arguments to function