Re: [PATCH] kvm memslot read-locking with mmu_lock

[Andrea Arcangeli <andrea-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

On Tue, Jan 22, 2008 at 03:47:28PM +0200, Avi Kivity wrote:
> Andrea Arcangeli wrote:
>> This adds locking to the memslots so they can be looked up with only
>> the mmu_lock. Entries with memslot->userspace_addr have to be ignored
>> because they're not fully inserted yet.
>>
>>   
> What is the motivation for this?  Calls from mmu notifiers that don't have 
> mmap_sem held?

Exactly.

>
>>   	/* Allocate page dirty bitmap if needed */
>> @@ -311,14 +320,18 @@ int __kvm_set_memory_region(struct kvm *kvm,
>>  		memset(new.dirty_bitmap, 0, dirty_bytes);
>>  	}
>>  +	spin_lock(&kvm->mmu_lock);
>>  	if (mem->slot >= kvm->nmemslots)
>>  		kvm->nmemslots = mem->slot + 1;
>>   	*memslot = new;
>> +	spin_unlock(&kvm->mmu_lock);
>>   	r = kvm_arch_set_memory_region(kvm, mem, old, user_alloc);
>>  	if (r) {
>> +		spin_lock(&kvm->mmu_lock);
>>  		*memslot = old;
>> +		spin_unlock(&kvm->mmu_lock);
>>  		goto out_free;
>>  	}
>>    
>
> This is arch independent code, I'm surprised mmu_lock is visible here?

The mmu_lock is arch independent as far as I can tell. Pretty much
like the mm->page_table_lock is also independent. All archs will have
some form of shadow pagetables in software or hardware, and mmu_lock
is the lock to take to serialize the pagetable updates and it also
allows to walk the memslots in readonly mode.

> What are the new lookup rules?  We don't hold mmu_lock everywhere we look 
> up a gfn, do we?

It's safe to loop over the memslots by just skipping the ones with
userland_addr == 0 by only holding the mmu_lock. The memslots contents
can't change by holding the mmu_lock. The mmu_lock also serializes the
rmap structures inside the memslot and the spte updates. So by just
taking the mmu_lock it's trivial to do "search memslot", "translate
the hva to its relative rmapp", "find all sptes relative to the hva
and overwrite them with nonpresent-fault".

If the mmu_notifiers would have been registered inside the vma things
would look very different in this area and it might have been possible
to embed the mmu-notifier inside the memslot itself, to avoid the
"search memslot" op.

-------------------------------------------------------------------------
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/