On 12/23/2009 04:19 PM, Marcelo Tosatti wrote:
On Wed, Dec 23, 2009 at 02:32:55PM +0200, Avi Kivity wrote:
On 12/23/2009 01:38 PM, Marcelo Tosatti wrote:
Use two steps for memslot deletion: mark the slot invalid (which stops
instantiation of new shadow pages for that slot, but allows destruction),
then instantiate the new empty slot.
Also simplifies kvm_handle_hva locking.
r = kvm_arch_prepare_memory_region(kvm,&new, old, user_alloc);
if (r)
goto out_free;
r == 0
Huh? kvm_arch_prepare_memory_region returns a suitable error code on
failure, 0 on success.
That's fine here, just pointing out you're inheriting this r == 0 later on.
kvm_iommu_map_pages returns 0 on success, error code otherwise? The
error code of kvm_iommu_map_pages was returned before the patchset, BTW.
Ok, then please fix it separately for easier backporting.
nr_mmu_pages = nr_pages * KVM_PERMILLE_MMU_PAGES / 1000;
nr_mmu_pages = max(nr_mmu_pages,
(unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
+ srcu_read_unlock(&kvm->srcu, idx);
Again, would like to move the srcu_locking to an outer scope.
This one was for documentation purposes only, since
kvm_mmu_calculate_mmu_pages is only called from commit_memory_region
(which mutually excludes itself).
I can remove it if you'd prefer.
A comment should be sufficient. Otherwise someone might read the code
and point out it's unnecessary.
Shouldn't we already hold the srcu_lock as part of normal guest exit
(similar to down_read() we do today)?
Yes, but:
kvm_arch_vcpu_setup -> vmx_vcpu_reset -> vmx_set_cr0 -> enter_pmode
So its called outside guest context path (memslot info is used outside
guest context in this case).
Didn't we take slots_lock previously? If so, that's a bug. We should
probably make all vcpu ioctls take slots_lock at top level, since many
vcpu ioctls can be caused to access memory.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
