RFC: switch vcpu context to use SRCU
cover RFC: switch vcpu context to use SRCU
Improves vmexit latency by ~= 13% on UP guest.
Still requires decent stress testing and careful review, but posting for early comments.
PATCH 1/10 KVM: modify memslots layout in struct kvm
Have a pointer to an allocated region inside struct kvm.
arch/x86/kvm/mmu.c +6 -5
@@ -768,13 +768,14 @@ static int kvm_handle_hva(struct kvm *kv {int i, j;int retval =0 ; +struct kvm_memslots *slots = kvm->memslots;/* * If mmap_sem isn't taken, we can look the memslots with only * the mmu_lock by skipping over the slots with userspace_addr == 0. */ -for (i =0 ; i < kvm->nmemslots; i++) { -struct kvm_memory_slot *memslot = &kvm->memslots[i]; +for (i =0 ; i < slots->nmemslots; i++) { +struct kvm_memory_slot *memslot = &slots->memslots[i];unsigned long start = memslot->userspace_addr;unsigned long end;
@@ -2969,8 +2970,8 @@ unsigned int kvm_mmu_calculate_mmu_pagesunsigned int nr_mmu_pages;unsigned int nr_pages =0 ; -for (i =0 ; i < kvm->nmemslots; i++) - nr_pages += kvm->memslots[i].npages; +for (i =0 ; i < kvm->memslots->nmemslots; i++) + nr_pages += kvm->memslots->memslots[i].npages; nr_mmu_pages = nr_pages *KVM_PERMILLE_MMU_PAGES /1000 ; nr_mmu_pages = max(nr_mmu_pages,
@@ -3243,7 +3244,7 @@ static int count_rmaps(struct kvm_vcpu *int i, j, k;for (i =0 ; i <KVM_MEMORY_SLOTS ; ++i) { -struct kvm_memory_slot *m = &vcpu->kvm->memslots[i]; +struct kvm_memory_slot *m = &vcpu->kvm->memslots->memslots[i];struct kvm_rmap_desc *d;for (j =0 ; j < m->npages; ++j) {
arch/x86/kvm/vmx.c +2 -2
@@ -1465,8 +1465,8 @@ static void enter_pmode(struct kvm_vcpustatic gva_t rmode_tss_base (struct kvm *kvm) {if (!kvm->arch.tss_addr) { -gfn_t base_gfn = kvm->memslots[0 ].base_gfn + - kvm->memslots[0 ].npages -3 ; +gfn_t base_gfn = kvm->memslots->memslots[0 ].base_gfn + + kvm->memslots->memslots[0 ].npages -3 ;return base_gfn <<PAGE_SHIFT ; }return kvm->arch.tss_addr;
arch/x86/kvm/x86.c +2 -2
@@ -2159,7 +2159,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvspin_lock (&kvm->mmu_lock);kvm_mmu_slot_remove_write_access (kvm, log->slot);spin_unlock (&kvm->mmu_lock); - memslot = &kvm->memslots[log->slot]; + memslot = &kvm->memslots->memslots[log->slot]; n =ALIGN (memslot->npages,BITS_PER_LONG ) /8 ;memset (memslot->dirty_bitmap,0 , n); }
@@ -4845,7 +4845,7 @@ int kvm_arch_set_memory_region(struct kv int user_alloc) {int npages = mem->memory_size >>PAGE_SHIFT ; -struct kvm_memory_slot *memslot = &kvm->memslots[mem->slot]; +struct kvm_memory_slot *memslot = &kvm->memslots->memslots[mem->slot];/*To keep backward compatibility with older userspace, *x86 needs to hanlde !user_alloc case.
include/linux/kvm_host.h +8 -4
@@ -151,14 +151,18 @@ struct kvm_irq_routing_table {};#endif +struct kvm_memslots { +int nmemslots; +struct kvm_memory_slot memslots[KVM_MEMORY_SLOTS + +KVM_PRIVATE_MEM_SLOTS ]; +}; +struct kvm {spinlock_t mmu_lock;spinlock_t requests_lock;struct rw_semaphore slots_lock;struct mm_struct *mm;/* userspace tied to this vm */ -int nmemslots; -struct kvm_memory_slot memslots[KVM_MEMORY_SLOTS + -KVM_PRIVATE_MEM_SLOTS ]; +struct kvm_memslots *memslots;#ifdef CONFIG_KVM_APIC_ARCHITECTURE u32 bsp_vcpu_id;struct kvm_vcpu *bsp_vcpu;
@@ -482,7 +486,7 @@ static inline void kvm_guest_exit(void)static inline int memslot_id (struct kvm *kvm,struct kvm_memory_slot *slot) { -return slot - kvm->memslots; +return slot - kvm->memslots->memslots; }static inline gpa_t gfn_to_gpa (gfn_t gfn)
virt/kvm/iommu.c +12 -6
@@ -76,10 +76,13 @@ unmap_pages:static int kvm_iommu_map_memslots (struct kvm *kvm) {int i, r =0 ; +struct kvm_memslots *slots; -for (i =0 ; i < kvm->nmemslots; i++) { - r =kvm_iommu_map_pages (kvm, kvm->memslots[i].base_gfn, - kvm->memslots[i].npages); + slots = kvm->memslots; + +for (i =0 ; i < slots->nmemslots; i++) { + r =kvm_iommu_map_pages (kvm, slots->memslots[i].base_gfn, + slots->memslots[i].npages);if (r)break ; }
@@ -210,10 +213,13 @@ static void kvm_iommu_put_pages(struct kstatic int kvm_iommu_unmap_memslots (struct kvm *kvm) {int i; +struct kvm_memslots *slots; + + slots = kvm->memslots; -for (i =0 ; i < kvm->nmemslots; i++) { -kvm_iommu_put_pages (kvm, kvm->memslots[i].base_gfn, - kvm->memslots[i].npages); +for (i =0 ; i < slots->nmemslots; i++) { +kvm_iommu_put_pages (kvm, slots->memslots[i].base_gfn, + slots->memslots[i].npages); }return 0 ;
virt/kvm/kvm_main.c +23 -13
@@ -348,12 +348,16 @@ static struct kvm *kvm_create_vm(void)INIT_HLIST_HEAD (&kvm->irq_ack_notifier_list);#endif + r = -ENOMEM ; + kvm->memslots =kzalloc (sizeof (struct kvm_memslots ),GFP_KERNEL ); +if (!kvm->memslots) + goto out_err; +#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET page =alloc_page (GFP_KERNEL | __GFP_ZERO); -if (!page) { - r = -ENOMEM ; +if (!page) goto out_err; - } + kvm->coalesced_mmio_ring = (struct kvm_coalesced_mmio_ring *)page_address (page);#endif
@@ -394,6 +398,7 @@ out: out_err:hardware_disable_all (); out_err_nodisable: +kfree (kvm->memslots);kfree (kvm);return ERR_PTR (r); }
@@ -428,9 +433,12 @@ static void kvm_free_physmem_slot(structvoid kvm_free_physmem (struct kvm *kvm) {int i; +struct kvm_memslots *slots = kvm->memslots; + +for (i =0 ; i < slots->nmemslots; ++i) +kvm_free_physmem_slot (&slots->memslots[i],NULL ); -for (i =0 ; i < kvm->nmemslots; ++i) -kvm_free_physmem_slot (&kvm->memslots[i],NULL ); +kfree (kvm->memslots); }static void kvm_destroy_vm (struct kvm *kvm)
@@ -514,7 +522,7 @@ int __kvm_set_memory_region(struct kvm *if (mem->guest_phys_addr + mem->memory_size < mem->guest_phys_addr) goto out; - memslot = &kvm->memslots[mem->slot]; + memslot = &kvm->memslots->memslots[mem->slot]; base_gfn = mem->guest_phys_addr >>PAGE_SHIFT ; npages = mem->memory_size >>PAGE_SHIFT ;
@@ -535,7 +543,7 @@ int __kvm_set_memory_region(struct kvm */* Check for overlaps */ r = -EEXIST ;for (i =0 ; i <KVM_MEMORY_SLOTS ; ++i) { -struct kvm_memory_slot *s = &kvm->memslots[i]; +struct kvm_memory_slot *s = &kvm->memslots->memslots[i];if (s == memslot || !s->npages)continue ;
@@ -637,8 +645,8 @@ skip_lpage:kvm_arch_flush_shadow (kvm);spin_lock (&kvm->mmu_lock); -if (mem->slot >= kvm->nmemslots) - kvm->nmemslots = mem->slot +1 ; +if (mem->slot >= kvm->memslots->nmemslots) + kvm->memslots->nmemslots = mem->slot +1 ; *memslot = new;spin_unlock (&kvm->mmu_lock);
@@ -708,7 +716,7 @@ int kvm_get_dirty_log(struct kvm *kvm,if (log->slot >=KVM_MEMORY_SLOTS ) goto out; - memslot = &kvm->memslots[log->slot]; + memslot = &kvm->memslots->memslots[log->slot]; r = -ENOENT ;if (!memslot->dirty_bitmap) goto out;
@@ -762,9 +770,10 @@ EXPORT_SYMBOL_GPL(kvm_is_error_hva);struct kvm_memory_slot *gfn_to_memslot_unaliased (struct kvm *kvm,gfn_t gfn) {int i; +struct kvm_memslots *slots = kvm->memslots; -for (i =0 ; i < kvm->nmemslots; ++i) { -struct kvm_memory_slot *memslot = &kvm->memslots[i]; +for (i =0 ; i < slots->nmemslots; ++i) { +struct kvm_memory_slot *memslot = &slots->memslots[i];if (gfn >= memslot->base_gfn && gfn < memslot->base_gfn + memslot->npages)
@@ -783,10 +792,11 @@ struct kvm_memory_slot *gfn_to_memslot(sint kvm_is_visible_gfn (struct kvm *kvm,gfn_t gfn) {int i; +struct kvm_memslots *slots = kvm->memslots; gfn =unalias_gfn (kvm, gfn);for (i =0 ; i <KVM_MEMORY_SLOTS ; ++i) { -struct kvm_memory_slot *memslot = &kvm->memslots[i]; +struct kvm_memory_slot *memslot = &slots->memslots[i];if (gfn >= memslot->base_gfn && gfn < memslot->base_gfn + memslot->npages)
PATCH 2/10 KVM: modify alias layout in x86s struct kvm_arch
In reply to Marcelo TosattiHave a pointer to an allocated region inside x86's kvm_arch.
arch/x86/include/asm/kvm_host.h +6 -2
@@ -379,9 +379,13 @@ struct kvm_mem_alias {gfn_t target_gfn; }; -struct kvm_arch { -int naliases; +struct kvm_mem_aliases {struct kvm_mem_alias aliases[KVM_ALIAS_SLOTS ]; +int naliases; +}; + +struct kvm_arch { +struct kvm_mem_aliases *aliases;unsigned int n_free_mmu_pages;unsigned int n_requested_mmu_pages;
arch/x86/kvm/x86.c +16 -5
@@ -1959,9 +1959,10 @@ gfn_t unalias_gfn(struct kvm *kvm, gfn_t {int i;struct kvm_mem_alias *alias; +struct kvm_mem_aliases *aliases = kvm->arch.aliases; -for (i =0 ; i < kvm->arch.naliases; ++i) { - alias = &kvm->arch.aliases[i]; +for (i =0 ; i < aliases->naliases; ++i) { + alias = &aliases->aliases[i];if (gfn >= alias->base_gfn && gfn < alias->base_gfn + alias->npages)return alias->target_gfn + gfn - alias->base_gfn;
@@ -1979,6 +1980,7 @@ static int kvm_vm_ioctl_set_memory_alias {int r, n;struct kvm_mem_alias *p; +struct kvm_mem_aliases *aliases; r = -EINVAL ;/* General sanity checks */
@@ -1998,15 +2000,17 @@ static int kvm_vm_ioctl_set_memory_aliasdown_write (&kvm->slots_lock);spin_lock (&kvm->mmu_lock); - p = &kvm->arch.aliases[alias->slot]; + aliases = kvm->arch.aliases; + + p = &aliases->aliases[alias->slot]; p->base_gfn = alias->guest_phys_addr >>PAGE_SHIFT ; p->npages = alias->memory_size >>PAGE_SHIFT ; p->target_gfn = alias->target_phys_addr >>PAGE_SHIFT ;for (n =KVM_ALIAS_SLOTS ; n >0 ; --n) -if (kvm->arch.aliases[n -1 ].npages) +if (aliases->aliases[n -1 ].npages)break ; - kvm->arch.naliases = n; + aliases->naliases = n;spin_unlock (&kvm->mmu_lock);kvm_mmu_zap_all (kvm);
@@ -4780,6 +4784,12 @@ struct kvm *kvm_arch_create_vm(void)if (!kvm)return ERR_PTR (-ENOMEM ); + kvm->arch.aliases =kzalloc (sizeof (struct kvm_mem_aliases ),GFP_KERNEL ); +if (!kvm->arch.aliases) { +kfree (kvm); +return ERR_PTR (-ENOMEM ); + } +INIT_LIST_HEAD (&kvm->arch.active_mmu_pages);INIT_LIST_HEAD (&kvm->arch.assigned_dev_head);
@@ -4836,6 +4846,7 @@ void kvm_arch_destroy_vm(struct kvm *kvmput_page (kvm->arch.apic_access_page);if (kvm->arch.ept_identity_pagetable)put_page (kvm->arch.ept_identity_pagetable); +kfree (kvm->arch.aliases);kfree (kvm); }
PATCH 3/10 KVM: switch dirty_log to mmu_lock protection
In reply to Marcelo Tosattiget_dirty_log vs mark_page_dirty need to be mutually exclusive. Switch to mmu_lock protection.
arch/x86/kvm/paging_tmpl.h +4 -0
@@ -175,7 +175,9 @@ walk:if (!(pte &PT_ACCESSED_MASK )) {trace_kvm_mmu_set_accessed_bit (table_gfn, index,sizeof (pte)); +spin_lock (&vcpu->kvm->mmu_lock);mark_page_dirty (vcpu->kvm, table_gfn); +spin_unlock (&vcpu->kvm->mmu_lock);if (FNAME (cmpxchg_gpte)(vcpu->kvm, table_gfn, index, pte, pte|PT_ACCESSED_MASK )) goto walk;
@@ -215,7 +217,9 @@ walk:bool ret;trace_kvm_mmu_set_dirty_bit (table_gfn, index,sizeof (pte)); +spin_lock (&vcpu->kvm->mmu_lock);mark_page_dirty (vcpu->kvm, table_gfn); +spin_unlock (&vcpu->kvm->mmu_lock); ret =FNAME (cmpxchg_gpte)(vcpu->kvm, table_gfn, index, pte, pte|PT_DIRTY_MASK );if (ret)
arch/x86/kvm/x86.c +31 -9
@@ -692,7 +692,9 @@ static void kvm_write_guest_time(structkunmap_atomic (shared_kaddr,KM_USER0 ); +spin_lock (&v->kvm->mmu_lock);mark_page_dirty (v->kvm, vcpu->time >>PAGE_SHIFT ); +spin_unlock (&v->kvm->mmu_lock); }static int kvm_request_guest_time_update (struct kvm_vcpu *v)
@@ -2147,27 +2149,45 @@ static int kvm_vm_ioctl_reinject(structint kvm_vm_ioctl_get_dirty_log (struct kvm *kvm,struct kvm_dirty_log *log) { -int r; -int n; +int r, n, i;struct kvm_memory_slot *memslot; -int is_dirty =0 ; +unsigned long is_dirty =0 ; +unsigned long *dirty_bitmap;down_write (&kvm->slots_lock); - r =kvm_get_dirty_log (kvm, log, &is_dirty); -if (r) + r = -EINVAL ; +if (log->slot >=KVM_MEMORY_SLOTS ) + goto out; + + memslot = &kvm->memslots->memslots[log->slot]; + r = -ENOENT ; +if (!memslot->dirty_bitmap) + goto out; + + n =ALIGN (memslot->npages,BITS_PER_LONG ) /8 ; + r = -ENOMEM ; + dirty_bitmap =vmalloc (n); +if (!dirty_bitmap) goto out; +memset (dirty_bitmap,0 , n); + +spin_lock (&kvm->mmu_lock); +for (i =0 ; !is_dirty && i < n/sizeof (long ); ++i) + is_dirty = memslot->dirty_bitmap[i];/* If nothing is dirty, don't bother messing with page tables. */ if (is_dirty) { -spin_lock (&kvm->mmu_lock); +memcpy (dirty_bitmap, memslot->dirty_bitmap, n); +memset (memslot->dirty_bitmap,0 , n);kvm_mmu_slot_remove_write_access (kvm, log->slot); -spin_unlock (&kvm->mmu_lock); memslot = &kvm->memslots->memslots[log->slot]; - n =ALIGN (memslot->npages,BITS_PER_LONG ) /8 ; -memset (memslot->dirty_bitmap,0 , n); } +spin_unlock (&kvm->mmu_lock); + r =0 ; +if (copy_to_user (log->dirty_bitmap, dirty_bitmap, n)) + r = -EFAULT ; out:up_write (&kvm->slots_lock);return r;
@@ -3491,7 +3511,9 @@ static void vapic_exit(struct kvm_vcpu *down_read (&vcpu->kvm->slots_lock);kvm_release_page_dirty (apic->vapic_page); +spin_lock (&vcpu->kvm->mmu_lock);mark_page_dirty (vcpu->kvm, apic->vapic_addr >>PAGE_SHIFT ); +spin_unlock (&vcpu->kvm->mmu_lock);up_read (&vcpu->kvm->slots_lock); }
virt/kvm/kvm_main.c +2 -0
@@ -1007,7 +1007,9 @@ int kvm_write_guest_page(struct kvm *kvm r =copy_to_user ((void __user *)addr + offset, data, len);if (r)return -EFAULT ; +spin_lock (&kvm->mmu_lock);mark_page_dirty (kvm, gfn); +spin_unlock (&kvm->mmu_lock);return 0 ; }EXPORT_SYMBOL_GPL (kvm_write_guest_page);
Re: [patch 03/10] KVM: switch dirty_log to mmu_lock protection
Quoted 4 lines · Marcelo Tosatti
get_dirty_logvsmark_page_dirtyneed to be mutually exclusive. Switch tommu_lockprotection.
I'm not sure all archs use mmu_lock. Also, I'm unhappy about introducing more use (especially as it's often unnecessary, if dirty logging is turned off for the slot for example).
I think you can use rcu for this as well. When you read the log, allocate a new empty bitmap, switch the memslots pointer to include it, and synchronize_srcu(). Now we are certain everyone is using the new bitmap we can copy the old one to usespace and delete it.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Re: [patch 03/10] KVM: switch dirty_log to mmu_lock protection
Quoted 9 lines · Avi Kivity
Quoted 4 lines · Marcelo Tosatti
get_dirty_logvsmark_page_dirtyneed to be mutually exclusive. Switch tommu_lockprotection.I'm not sure all archs use
mmu_lock. Also, I'm unhappy about introducing more use (especially as it's often unnecessary, if dirty logging is turned off for the slot for example).
True.
Quoted 4 lines
I think you can use rcu for this as well. When you read the log, allocate a new empty bitmap, switch the memslots pointer to include it, and
synchronize_srcu(). Now we are certain everyone is using the new bitmap we can copy the old one to usespace and delete it.
This will slow down get_dirty_log, which can lead to regressions in migration and poorer vga refresh updates.
Moreover, since updates are running concurrently, at the time synchronize_srcu returns you can have plenty of dirty data in the new bitmap which is not the case with mutual exclusion.
This are the reasons for the choice. Would you prefer a new rw lock?
Re: [patch 03/10] KVM: switch dirty_log to mmu_lock protection
Quoted 9 lines · Marcelo Tosatti
Quoted 6 lines
I think you can use rcu for this as well. When you read the log, allocate a new empty bitmap, switch the memslots pointer to include it, and
synchronize_srcu(). Now we are certain everyone is using the new bitmap we can copy the old one to usespace and delete it.This will slow down
get_dirty_log, which can lead to regressions in migration and poorer vga refresh updates.
That's true. By how much? srcu is not so slow that a 30Hz refresh rate would notice.
Quoted 4 lines
Moreover, since updates are running concurrently, at the time
synchronize_srcureturns you can have plenty of dirty data in the new bitmap which is not the case with mutual exclusion.
By the time ioctl() returns you can have dirty data in the new bitmap. Nothing wrong with that.
Quoted 2 lines
This are the reasons for the choice. Would you prefer a new rw lock?
Prefer the srcu thing unless convinced it is too slow. Alternatively, we can copy-and-clear the bitmap using cmpxchg (which won't lose data) to a temporary buffer, then to userspace. Locks are unfashionable.
Signature · 1 line
error compiling committee.c: too many arguments to function
PATCH 4/10 KVM: split kvm_arch_set_memory_region into prepare and commit
In reply to Marcelo TosattiRequired for SRCU convertion later.
arch/ia64/kvm/kvm-ia64.c +11 -4
@@ -1576,15 +1576,14 @@ out:return r; } -int kvm_arch_set_memory_region (struct kvm *kvm, -struct kvm_userspace_memory_region *mem, +int kvm_arch_prepare_memory_region (struct kvm *kvm, +struct kvm_memory_slot *memslot,struct kvm_memory_slot old,int user_alloc) {unsigned long i;unsigned long pfn; -int npages = mem->memory_size >>PAGE_SHIFT ; -struct kvm_memory_slot *memslot = &kvm->memslots[mem->slot]; +int npages = memslot->npages;unsigned long base_gfn = memslot->base_gfn;if (base_gfn + npages > (KVM_MAX_MEM_SIZE >>PAGE_SHIFT ))
@@ -1608,6 +1607,14 @@ int kvm_arch_set_memory_region(struct kvreturn 0 ; } +void kvm_arch_commit_memory_region (struct kvm *kvm, +struct kvm_userspace_memory_region *mem, +struct kvm_memory_slot old, +int user_alloc) +{ +return ; +} +void kvm_arch_flush_shadow (struct kvm *kvm) {kvm_flush_remote_tlbs (kvm);
arch/s390/kvm/kvm-s390.c +12 -6
@@ -680,10 +680,10 @@ long kvm_arch_vcpu_ioctl(struct file *fi }/* Section: memory related */ -int kvm_arch_set_memory_region (struct kvm *kvm, -struct kvm_userspace_memory_region *mem, -struct kvm_memory_slot old, -int user_alloc) +int kvm_arch_prepare_memory_region (struct kvm *kvm, +struct kvm_userspace_memory_region *mem, +struct kvm_memory_slot old, +int user_alloc) {int i;struct kvm_vcpu *vcpu;
@@ -710,14 +710,20 @@ int kvm_arch_set_memory_region(struct kvif (!user_alloc)return -EINVAL ; +return 0 ; +} + +int kvm_arch_commit_memory_region (struct kvm *kvm, +struct kvm_userspace_memory_region *mem, +struct kvm_memory_slot old, +int user_alloc) +{/* request update of sie control block for all available vcpus */ kvm_for_each_vcpu (i, vcpu, kvm) {if (test_and_set_bit (KVM_REQ_MMU_RELOAD , &vcpu->requests))continue ;kvm_s390_inject_sigp_stop (vcpu,ACTION_RELOADVCPU_ON_STOP ); } - -return 0 ; }void kvm_arch_flush_shadow (struct kvm *kvm)
arch/x86/kvm/x86.c +28 -22
75 lines not on this page · raw message
include/linux/kvm_host.h +5 -1
@@ -254,7 +254,11 @@ int kvm_set_memory_region(struct kvm *kvint __kvm_set_memory_region (struct kvm *kvm,struct kvm_userspace_memory_region *mem,int user_alloc); -int kvm_arch_set_memory_region (struct kvm *kvm, +int kvm_arch_prepare_memory_region (struct kvm *kvm, +struct kvm_memory_slot *memslot, +struct kvm_memory_slot old, +int user_alloc); +void kvm_arch_commit_memory_region (struct kvm *kvm,struct kvm_userspace_memory_region *mem,struct kvm_memory_slot old,int user_alloc);
virt/kvm/kvm_main.c +5 -7
@@ -644,6 +644,10 @@ skip_lpage:if (!npages)kvm_arch_flush_shadow (kvm); + r =kvm_arch_prepare_memory_region (kvm, &new, old, user_alloc); +if (r) + goto out_free; +spin_lock (&kvm->mmu_lock);if (mem->slot >= kvm->memslots->nmemslots) kvm->memslots->nmemslots = mem->slot +1 ;
@@ -651,13 +655,7 @@ skip_lpage: *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; - } +kvm_arch_commit_memory_region (kvm, mem, old, user_alloc);kvm_free_physmem_slot (&old, npages ? &new :NULL );/* Slot deletion case: we have to update the current slot */
Re: [patch 04/10] KVM: split kvm_arch_set_memory_region into prepare and commit
Quoted 3 lines · Marcelo Tosatti
Required for SRCU convertion later.
My feeling is that prepare/commit can be common code and only some small bits (like allocating x86 rmaps) can be arch specific. That's for another patchset though.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
PATCH 5/10 KVM: introduce gfn_to_pfn_memslot
In reply to Marcelo TosattiWhich takes a memslot pointer instead of using kvm->memslots.
To be used by SRCU convertion later.
include/linux/kvm_host.h +2 -0
@@ -273,6 +273,8 @@ void kvm_set_page_dirty(struct page *pagvoid kvm_set_page_accessed (struct page *page);pfn_t gfn_to_pfn (struct kvm *kvm,gfn_t gfn); +pfn_t gfn_to_pfn_memslot (struct kvm *kvm, +struct kvm_memory_slot *slot,gfn_t gfn);void kvm_release_pfn_dirty (pfn_t );void kvm_release_pfn_clean (pfn_t pfn);void kvm_set_pfn_dirty (pfn_t pfn);
virt/kvm/kvm_main.c +25 -8
@@ -816,21 +816,14 @@ unsigned long gfn_to_hva(struct kvm *kvm }EXPORT_SYMBOL_GPL (gfn_to_hva); -pfn_t gfn_to_pfn (struct kvm *kvm,gfn_t gfn) +static pfn_t __gfn_to_pfn (struct kvm *kvm,unsigned long addr) {struct page *page[1 ]; -unsigned long addr;int npages;pfn_t pfn;might_sleep (); - addr =gfn_to_hva (kvm, gfn); -if (kvm_is_error_hva (addr)) { -get_page (bad_page); -return page_to_pfn (bad_page); - } - npages =get_user_pages_fast (addr,1 ,1 , page);if (unlikely (npages !=1 )) {
@@ -855,8 +848,32 @@ pfn_t gfn_to_pfn(struct kvm *kvm, gfn_treturn pfn; } +pfn_t gfn_to_pfn (struct kvm *kvm,gfn_t gfn) +{ +unsigned long addr; + + addr =gfn_to_hva (kvm, gfn); +if (kvm_is_error_hva (addr)) { +get_page (bad_page); +return page_to_pfn (bad_page); + } + +return __gfn_to_pfn (kvm, addr); +}EXPORT_SYMBOL_GPL (gfn_to_pfn); +static unsigned long gfn_to_hva_memslot (struct kvm_memory_slot *slot,gfn_t gfn) +{ +return (slot->userspace_addr + (gfn - slot->base_gfn) *PAGE_SIZE ); +} + +pfn_t gfn_to_pfn_memslot (struct kvm *kvm, +struct kvm_memory_slot *slot,gfn_t gfn) +{ +unsigned long addr =gfn_to_hva_memslot (slot, gfn); +return __gfn_to_pfn (kvm, addr); +} +struct page *gfn_to_page (struct kvm *kvm,gfn_t gfn) {pfn_t pfn;
PATCH 6/10 KVM: use gfn_to_pfn_memslot in kvm_iommu_map_pages
In reply to Marcelo TosattiSo its possible to iommu map a memslot before making it visible to kvm.
include/linux/kvm_host.h +1 -2
@@ -439,8 +439,7 @@ void kvm_free_irq_source_id(struct kvm *#define KVM_IOMMU_CACHE_COHERENCY 0x1#ifdef CONFIG_IOMMU_API -int kvm_iommu_map_pages (struct kvm *kvm,gfn_t base_gfn, -unsigned long npages); +int kvm_iommu_map_pages (struct kvm *kvm,struct kvm_memory_slot *slot);int kvm_iommu_map_guest (struct kvm *kvm);int kvm_iommu_unmap_guest (struct kvm *kvm); int kvm_assign_device(struct kvm *kvm,
virt/kvm/iommu.c +6 -7
@@ -32,10 +32,10 @@ static int kvm_iommu_unmap_memslots(strustatic void kvm_iommu_put_pages (struct kvm *kvm,gfn_t base_gfn,unsigned long npages); -int kvm_iommu_map_pages (struct kvm *kvm, -gfn_t base_gfn,unsigned long npages) +int kvm_iommu_map_pages (struct kvm *kvm,struct kvm_memory_slot *slot) { -gfn_t gfn = base_gfn; +gfn_t gfn = slot->base_gfn; +unsigned long npages = slot->npages;pfn_t pfn;int i, r =0 ;struct iommu_domain *domain = kvm->arch.iommu_domain;
@@ -54,7 +54,7 @@ int kvm_iommu_map_pages(struct kvm *kvm,if (iommu_iova_to_phys (domain,gfn_to_gpa (gfn)))continue ; - pfn =gfn_to_pfn (kvm, gfn); + pfn =gfn_to_pfn_memslot (kvm, slot, gfn); r = iommu_map_range(domain, gfn_to_gpa(gfn), pfn_to_hpa(pfn),
@@ -69,7 +69,7 @@ int kvm_iommu_map_pages(struct kvm *kvm,return 0 ; unmap_pages: -kvm_iommu_put_pages (kvm, base_gfn, i); +kvm_iommu_put_pages (kvm, slot->base_gfn, i);return r; }
@@ -81,8 +81,7 @@ static int kvm_iommu_map_memslots(struct slots = kvm->memslots;for (i =0 ; i < slots->nmemslots; i++) { - r =kvm_iommu_map_pages (kvm, slots->memslots[i].base_gfn, - slots->memslots[i].npages); + r =kvm_iommu_map_pages (kvm, &slots->memslots[i]);if (r)break ; }
virt/kvm/kvm_main.c +1 -1
@@ -665,7 +665,7 @@ skip_lpage:spin_unlock (&kvm->mmu_lock);#ifdef CONFIG_DMAR /* map the pages in iommu page table */ - r =kvm_iommu_map_pages (kvm, base_gfn, npages); + r =kvm_iommu_map_pages (kvm, memslot);if (r) goto out;#endif
PATCH 7/10 KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update
In reply to Marcelo TosattiUse 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.
include/linux/kvm.h +1 -1
@@ -39,7 +39,7 @@ struct kvm_userspace_memory_region {/* for kvm_memory_region::flags */ #define KVM_MEM_LOG_DIRTY_PAGES 1UL - +#define KVM_MEMSLOT_INVALID (1UL << 1)/* for KVM_IRQ_LINE */ struct kvm_irq_level {
virt/kvm/kvm_main.c +72 -33
196 lines not on this page · raw message
arch/ia64/kvm/kvm-ia64.c +2 -0
@@ -1834,6 +1834,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvstruct kvm_memory_slot *memslot;int is_dirty =0 ; +down_write (&kvm->slots_lock);spin_lock (&kvm->arch.dirty_log_lock); r =kvm_ia64_sync_dirty_log (kvm, log);
@@ -1853,6 +1854,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kv } r =0 ; out: +up_write (&kvm->slots_lock);spin_unlock (&kvm->arch.dirty_log_lock);return r; }
arch/x86/kvm/mmu.c +19 -15
@@ -29,6 +29,7 @@#include <linux/swap.h> #include <linux/hugetlb.h> #include <linux/compiler.h> +#include <linux/srcu.h> #include <asm/page.h> #include <asm/cmpxchg.h>
@@ -766,23 +767,18 @@ static int kvm_unmap_rmapp(struct kvm *kstatic int kvm_handle_hva (struct kvm *kvm,unsigned long hva,int (*handler)(struct kvm *kvm,unsigned long *rmapp)) { -int i, j; +int i, j, idx;int retval =0 ; -struct kvm_memslots *slots = kvm->memslots; +struct kvm_memslots *slots; + + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots); -/* -* If mmap_sem isn't taken, we can look the memslots with only -* the mmu_lock by skipping over the slots with userspace_addr == 0. -*/ for (i =0 ; i < slots->nmemslots; i++) {struct kvm_memory_slot *memslot = &slots->memslots[i];unsigned long start = memslot->userspace_addr;unsigned long end; -/* mmu_lock protects userspace_addr */ -if (!start) -continue ; - end = start + (memslot->npages <<PAGE_SHIFT );if (hva >= start && hva < end) {gfn_t gfn_offset = (hva - start) >>PAGE_SHIFT ;
@@ -797,6 +793,7 @@ static int kvm_handle_hva(struct kvm *kv } } } +srcu_read_unlock (&kvm->srcu, idx);return retval; }
@@ -2966,16 +2963,20 @@ nomem:*/ unsigned int kvm_mmu_calculate_mmu_pages (struct kvm *kvm) { -int i; +int i, idx;unsigned int nr_mmu_pages;unsigned int nr_pages =0 ; +struct kvm_memslots *slots; -for (i =0 ; i < kvm->memslots->nmemslots; i++) - nr_pages += kvm->memslots->memslots[i].npages; + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots); +for (i =0 ; i < slots->nmemslots; i++) + nr_pages += slots->memslots[i].npages; 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);return nr_mmu_pages; }
@@ -3241,10 +3242,12 @@ static void audit_mappings(struct kvm_vcstatic int count_rmaps (struct kvm_vcpu *vcpu) {int nmaps =0 ; -int i, j, k; +int i, j, k, idx; + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots);for (i =0 ; i <KVM_MEMORY_SLOTS ; ++i) { -struct kvm_memory_slot *m = &vcpu->kvm->memslots->memslots[i]; +struct kvm_memory_slot *m = &slots->memslots[i];struct kvm_rmap_desc *d;for (j =0 ; j < m->npages; ++j) {
@@ -3267,6 +3270,7 @@ static int count_rmaps(struct kvm_vcpu * } } } +srcu_read_unlock (&kvm->srcu, idx);return nmaps; }
arch/x86/kvm/vmx.c +11 -2
@@ -24,6 +24,7 @@#include <linux/mm.h> #include <linux/highmem.h> #include <linux/sched.h> +#include <linux/srcu.h> #include <linux/moduleparam.h> #include <linux/ftrace_event.h> #include "kvm_cache_regs.h"
@@ -1465,10 +1466,18 @@ static void enter_pmode(struct kvm_vcpustatic gva_t rmode_tss_base (struct kvm *kvm) {if (!kvm->arch.tss_addr) { -gfn_t base_gfn = kvm->memslots->memslots[0 ].base_gfn + - kvm->memslots->memslots[0 ].npages -3 ; +struct kvm_memslots *slots; +gfn_t base_gfn; +int idx; + + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots); + base_gfn = slots->memslots[0 ].base_gfn + + slots->memslots[0 ].npages -3 ; +srcu_read_unlock (&kvm->srcu, idx);return base_gfn <<PAGE_SHIFT ; } +return kvm->arch.tss_addr; }
include/linux/kvm_host.h +1 -0
@@ -163,6 +163,7 @@ struct kvm {struct rw_semaphore slots_lock;struct mm_struct *mm;/* userspace tied to this vm */ struct kvm_memslots *memslots; +struct srcu_struct srcu;#ifdef CONFIG_KVM_APIC_ARCHITECTURE u32 bsp_vcpu_id;struct kvm_vcpu *bsp_vcpu;
virt/kvm/assigned-dev.c +4 -4
@@ -504,11 +504,11 @@ out:static int kvm_vm_ioctl_assign_device (struct kvm *kvm,struct kvm_assigned_pci_dev *assigned_dev) { -int r =0 ; +int r =0 , idx;struct kvm_assigned_dev_kernel *match;struct pci_dev *dev; -down_read (&kvm->slots_lock); + idx =srcu_read_lock (&kvm->srcu);mutex_lock (&kvm->lock); match = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,
@@ -574,7 +574,7 @@ static int kvm_vm_ioctl_assign_device(st out:mutex_unlock (&kvm->lock); -up_read (&kvm->slots_lock); +srcu_read_unlock (&kvm->srcu, idx);return r; out_list_del:list_del (&match->list);
@@ -586,7 +586,7 @@ out_put: out_free:kfree (match);mutex_unlock (&kvm->lock); -up_read (&kvm->slots_lock); +srcu_read_unlock (&kvm->srcu, idx);return r; }
virt/kvm/iommu.c +2 -2
@@ -78,7 +78,7 @@ static int kvm_iommu_map_memslots(structint i, r =0 ;struct kvm_memslots *slots; - slots = kvm->memslots; + slots =rcu_dereference (kvm->memslots);for (i =0 ; i < slots->nmemslots; i++) { r =kvm_iommu_map_pages (kvm, &slots->memslots[i]);
@@ -214,7 +214,7 @@ static int kvm_iommu_unmap_memslots(struint i;struct kvm_memslots *slots; - slots = kvm->memslots; + slots =rcu_dereference (kvm->memslots);for (i =0 ; i < slots->nmemslots; i++) { kvm_iommu_put_pages(kvm, slots->memslots[i].base_gfn,
Re: [patch 07/10] KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update
Quoted 9 lines · Marcelo Tosatti
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_hvalocking.unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm) {
arch/x86/kvm/mmu.c nomem: · line 2971
Quoted 9 lines
arch/x86/kvm/mmu.c +3
-3-int i; +int i, idx;unsigned int nr_mmu_pages;unsigned int nr_pages =0 ; +struct kvm_memslots *slots; -for (i =0 ; i< kvm->memslots->nmemslots; i++) - nr_pages += kvm->memslots->memslots[i].npages; + idx =srcu_read_lock (&kvm->srcu);
Doesn't the caller hold the srcu_read_lock() here?
Quoted 22 lines
Index:
kvm-slotslock/arch/x86/kvm/vmx.c
===================================================================arch/x86/kvm/vmx.c +4
-2@@ -24,6 +24,7 @@#include <linux/mm.h> #include <linux/highmem.h> #include <linux/sched.h> +#include <linux/srcu.h> #include <linux/moduleparam.h> #include <linux/ftrace_event.h> #include "kvm_cache_regs.h" @@ -1465,10 +1466,18 @@ static void enter_pmode(struct kvm_vcpustatic gva_t rmode_tss_base (struct kvm *kvm) {if (!kvm->arch.tss_addr) { -gfn_t base_gfn = kvm->memslots->memslots[0 ].base_gfn + - kvm->memslots->memslots[0 ].npages -3 ; +struct kvm_memslots *slots; +gfn_t base_gfn; +int idx;
arch/x86/kvm/vmx.c static void enter_pmode(struct kvm_vcpu · line 1481
Quoted 10 lines
arch/x86/kvm/vmx.c +7
-0+ + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots); + base_gfn = slots->memslots[0 ].base_gfn + + slots->memslots[0 ].npages -3 ; +srcu_read_unlock (&kvm->srcu, idx);return base_gfn<<PAGE_SHIFT ; } +
And here? Maybe we should take the srcu_lock in vcpu_load/put and only drop in when going into vcpu context or explicitly sleeping, just to simplify things.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Re: [patch 07/10] KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update
Quoted 15 lines · Avi Kivity
Quoted 14 lines · Marcelo Tosatti
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_hvalocking.unsigned int kvm_mmu_calculate_mmu_pages(struct kvm *kvm) {arch/x86/kvm/mmu.c +2
-1-int i; +int i, idx;unsigned int nr_mmu_pages;unsigned int nr_pages =0 ; +struct kvm_memslots *slots;
Quoted 6 lines
arch/x86/kvm/mmu.c nomem: · line 2971
Quoted 4 lines
arch/x86/kvm/mmu.c +1
-2-for (i =0 ; i< kvm->memslots->nmemslots; i++) - nr_pages += kvm->memslots->memslots[i].npages; + idx =srcu_read_lock (&kvm->srcu);Doesn't the caller hold the
srcu_read_lock()here?
No:
kvm_vm_ioctl_set_nr_mmu_pages -> kvm_mmu_change_mmu_pages
And even if the caller did, recursive "locking" is tolerated.
Quoted 25 lines
Quoted 25 lines
Index:
kvm-slotslock/arch/x86/kvm/vmx.c
===================================================================arch/x86/kvm/vmx.c +8
-2@@ -24,6 +24,7 @@#include <linux/mm.h> #include <linux/highmem.h> #include <linux/sched.h> +#include <linux/srcu.h> #include <linux/moduleparam.h> #include <linux/ftrace_event.h> #include "kvm_cache_regs.h" @@ -1465,10 +1466,18 @@ static void enter_pmode(struct kvm_vcpustatic gva_t rmode_tss_base (struct kvm *kvm) {if (!kvm->arch.tss_addr) { -gfn_t base_gfn = kvm->memslots->memslots[0 ].base_gfn + - kvm->memslots->memslots[0 ].npages -3 ; +struct kvm_memslots *slots; +gfn_t base_gfn; +int idx; + + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots); + base_gfn = slots->memslots[0].base_gfn +
Quoted 8 lines
arch/x86/kvm/vmx.c static void enter_pmode(struct kvm_vcpu · line 1483
Quoted 6 lines
arch/x86/kvm/vmx.c +3
-0+ slots->memslots[0 ].npages -3 ; +srcu_read_unlock (&kvm->srcu, idx);return base_gfn<<PAGE_SHIFT ; } +And here?
kvm_arch_vcpu_ioctl_set_sregs -> kvm_x86_ops->set_cr0.
Quoted 3 lines
Maybe we should take the
srcu_lockin vcpu_load/put and only drop in when going into vcpu context or explicitly sleeping, just to simplify things.
Hum, possible, but i'd rather leave it for later.
Re: [patch 07/10] KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update
In reply to Marcelo TosattiResending with Cc: added
Quoted 111 lines · Marcelo Tosatti
plain text document attachment (introduce-srcu-and-use-for-slots) 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_hvalocking.Signed-off-by Marcelo Tosatti <[email protected]>Index:
kvm-slotslock/include/linux/kvm.h
===================================================================include/linux/kvm.h +1
-1@@ -39,7 +39,7 @@ struct kvm_userspace_memory_region {/* for kvm_memory_region::flags */ #define KVM_MEM_LOG_DIRTY_PAGES 1UL - +#define KVM_MEMSLOT_INVALID (1UL << 1)/* for KVM_IRQ_LINE */ struct kvm_irq_level {Index:
kvm-slotslock/virt/kvm/kvm_main.c
===================================================================virt/kvm/kvm_main.c +13
-12@@ -43,6 +43,7 @@#include <linux/swap.h> #include <linux/bitops.h> #include <linux/spinlock.h> +#include <linux/srcu.h> #include <asm/processor.h> #include <asm/io.h> @@ -352,11 +353,15 @@ static struct kvm *kvm_create_vm(void) kvm->memslots =kzalloc (sizeof (struct kvm_memslots ),GFP_KERNEL );if (!kvm->memslots) goto out_err; +if (init_srcu_struct (&kvm->srcu)) + goto out_err;#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET page =alloc_page (GFP_KERNEL | __GFP_ZERO); -if (!page) +if (!page) { +cleanup_srcu_struct (&kvm->srcu); goto out_err; + } kvm->coalesced_mmio_ring = (struct kvm_coalesced_mmio_ring *)page_address (page);@@ -367,6 +372,7 @@ static struct kvm *kvm_create_vm(void) kvm->mmu_notifier.ops = &kvm_mmu_notifier_ops; r =mmu_notifier_register (&kvm->mmu_notifier, current->mm);if (r) { +cleanup_srcu_struct (&kvm->srcu);#ifdef KVM_COALESCED_MMIO_PAGE_OFFSET put_page (page);#endif @@ -462,6 +468,7 @@ static void kvm_destroy_vm(struct kvm *kkvm_arch_flush_shadow (kvm);#endif kvm_arch_destroy_vm (kvm); +cleanup_srcu_struct (&kvm->srcu);hardware_disable_all ();mmdrop (mm); }@@ -502,12 +509,13 @@ int __kvm_set_memory_region(struct kvm *struct kvm_userspace_memory_region *mem, int user_alloc) { - int r; + int r, flush_shadow = 0;gfn_t base_gfn;unsigned long npages;unsigned long i;struct kvm_memory_slot *memslot;struct kvm_memory_slot old, new; +struct kvm_memslots *slots, *old_memslots; r = -EINVAL ;/* General sanity checks */ @@ -569,15 +577,7 @@ int __kvm_set_memory_region(struct kvm *memset (new.rmap,0 , npages *sizeof (*new.rmap)); new.user_alloc = user_alloc; -/* -* hva_to_rmmap() serialzies with the mmu_lock and to be -* safe it has to ignore memslots with !user_alloc && -* !userspace_addr. -*/ -if (user_alloc) - new.userspace_addr = mem->userspace_addr; -else - new.userspace_addr =0 ; + new.userspace_addr = mem->userspace_addr; }if (!npages) goto skip_lpage;@@ -632,8 +632,9 @@ skip_lpage:if (!new.dirty_bitmap) goto out_free;memset (new.dirty_bitmap,0 , dirty_bytes); +/* destroy any largepage mappings for dirty tracking */ if (old.npages) - kvm_arch_flush_shadow(kvm); + flush_shadow = 1; }#else /* not defined CONFIG_S390 */ new.user_alloc = user_alloc;
virt/kvm/kvm_main.c skip_lpage: · line 649
Quoted 9 lines
virt/kvm/kvm_main.c +5
-1@@ -641,34 +642,69 @@ skip_lpage: new.userspace_addr = mem->userspace_addr;#endif /* not defined CONFIG_S390 */ -if (!npages) +if (!npages) { + slots =kzalloc (sizeof (struct kvm_memslots ),GFP_KERNEL ); +if (!slots) + goto out_free; +memcpy (slots, kvm->memslots,sizeof (struct kvm_memslots ));
Nothing wrong with the above line, but it makes me think if
*slots = *kvm->memslots;
would save us the function call overhead
Quoted 29 lines
virt/kvm/kvm_main.c +20
-3+if (mem->slot >= slots->nmemslots) + slots->nmemslots = mem->slot +1 ; + slots->memslots[mem->slot].flags |=KVM_MEMSLOT_INVALID ; + + old_memslots = kvm->memslots; +rcu_assign_pointer (kvm->memslots, slots); +synchronize_srcu (&kvm->srcu); +/* 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); +kfree (old_memslots); + } r =kvm_arch_prepare_memory_region (kvm, &new, old, user_alloc);if (r) goto out_free; -spin_lock (&kvm->mmu_lock); -if (mem->slot >= kvm->memslots->nmemslots) - kvm->memslots->nmemslots = mem->slot +1 ; +#ifdef CONFIG_DMAR +/* map the pages in iommu page table */ +if (npages) + r =kvm_iommu_map_pages (kvm, &new);
virt/kvm/kvm_main.c skip_lpage: · line 683
Quoted 10 lines
virt/kvm/kvm_main.c +7
-2+if (r) + goto out_free; +#endif - *memslot = new; -spin_unlock (&kvm->mmu_lock); + slots =kzalloc (sizeof (struct kvm_memslots ),GFP_KERNEL ); +if (!slots) + goto out_free; +memcpy (slots, kvm->memslots,sizeof (struct kvm_memslots ));
Ditto
Cheers,
Fernando Carrijo.
Quoted 299 lines
virt/kvm/kvm_main.c +27
-15+if (mem->slot >= slots->nmemslots) + slots->nmemslots = mem->slot +1 ; + +/* actual memory is freed via old in kvm_free_physmem_slot below */ +if (!npages) { + new.rmap =NULL ; + new.dirty_bitmap =NULL ; +for (i =0 ; i <KVM_NR_PAGE_SIZES -1 ; ++i) + new.lpage_info[i] =NULL ; + } + + slots->memslots[mem->slot] = new; + old_memslots = kvm->memslots; +rcu_assign_pointer (kvm->memslots, slots); +synchronize_srcu (&kvm->srcu);kvm_arch_commit_memory_region (kvm, mem, old, user_alloc); -kvm_free_physmem_slot (&old, npages ? &new :NULL ); -/* Slot deletion case: we have to update the current slot */ -spin_lock (&kvm->mmu_lock); -if (!npages) - *memslot = old; -spin_unlock (&kvm->mmu_lock); -#ifdef CONFIG_DMAR -/* map the pages in iommu page table */ - r =kvm_iommu_map_pages (kvm, memslot); -if (r) - goto out; -#endif +kvm_free_physmem_slot (&old, &new); +kfree (old_memslots); + +if (flush_shadow) +kvm_arch_flush_shadow (kvm); +return 0 ; out_free:@@ -768,7 +804,7 @@ EXPORT_SYMBOL_GPL(kvm_is_error_hva);struct kvm_memory_slot *gfn_to_memslot_unaliased (struct kvm *kvm,gfn_t gfn) {int i; -struct kvm_memslots *slots = kvm->memslots; +struct kvm_memslots *slots =rcu_dereference (kvm->memslots);for (i =0 ; i < slots->nmemslots; ++i) {struct kvm_memory_slot *memslot = &slots->memslots[i];@@ -790,12 +826,15 @@ struct kvm_memory_slot *gfn_to_memslot(sint kvm_is_visible_gfn (struct kvm *kvm,gfn_t gfn) {int i; -struct kvm_memslots *slots = kvm->memslots; +struct kvm_memslots *slots =rcu_dereference (kvm->memslots); gfn =unalias_gfn (kvm, gfn);for (i =0 ; i <KVM_MEMORY_SLOTS ; ++i) {struct kvm_memory_slot *memslot = &slots->memslots[i]; +if (memslot->flags &KVM_MEMSLOT_INVALID ) +continue ; +if (gfn >= memslot->base_gfn && gfn < memslot->base_gfn + memslot->npages)return 1 ;@@ -810,7 +849,7 @@ unsigned long gfn_to_hva(struct kvm *kvm gfn =unalias_gfn (kvm, gfn); slot =gfn_to_memslot_unaliased (kvm, gfn); -if (!slot) +if (!slot || slot->flags &KVM_MEMSLOT_INVALID )return bad_hva ();return (slot->userspace_addr + (gfn - slot->base_gfn) *PAGE_SIZE ); }Index:
kvm-slotslock/arch/ia64/kvm/kvm-ia64.c
===================================================================arch/ia64/kvm/kvm-ia64.c +2
-0@@ -1834,6 +1834,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvstruct kvm_memory_slot *memslot;int is_dirty =0 ; +down_write (&kvm->slots_lock);spin_lock (&kvm->arch.dirty_log_lock); r =kvm_ia64_sync_dirty_log (kvm, log);@@ -1853,6 +1854,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kv } r =0 ; out: +up_write (&kvm->slots_lock);spin_unlock (&kvm->arch.dirty_log_lock);return r; }Index:
kvm-slotslock/arch/x86/kvm/mmu.c
===================================================================arch/x86/kvm/mmu.c +19
-15@@ -29,6 +29,7 @@#include <linux/swap.h> #include <linux/hugetlb.h> #include <linux/compiler.h> +#include <linux/srcu.h> #include <asm/page.h> #include <asm/cmpxchg.h> @@ -766,23 +767,18 @@ static int kvm_unmap_rmapp(struct kvm *kstatic int kvm_handle_hva (struct kvm *kvm,unsigned long hva,int (*handler)(struct kvm *kvm,unsigned long *rmapp)) { -int i, j; +int i, j, idx;int retval =0 ; -struct kvm_memslots *slots = kvm->memslots; +struct kvm_memslots *slots; + + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots); -/* -* If mmap_sem isn't taken, we can look the memslots with only -* the mmu_lock by skipping over the slots with userspace_addr == 0. -*/ for (i =0 ; i < slots->nmemslots; i++) {struct kvm_memory_slot *memslot = &slots->memslots[i];unsigned long start = memslot->userspace_addr;unsigned long end; -/* mmu_lock protects userspace_addr */ -if (!start) -continue ; - end = start + (memslot->npages <<PAGE_SHIFT );if (hva >= start && hva < end) {gfn_t gfn_offset = (hva - start) >>PAGE_SHIFT ;@@ -797,6 +793,7 @@ static int kvm_handle_hva(struct kvm *kv } } } +srcu_read_unlock (&kvm->srcu, idx);return retval; }@@ -2966,16 +2963,20 @@ nomem:*/ unsigned int kvm_mmu_calculate_mmu_pages (struct kvm *kvm) { -int i; +int i, idx;unsigned int nr_mmu_pages;unsigned int nr_pages =0 ; +struct kvm_memslots *slots; -for (i =0 ; i < kvm->memslots->nmemslots; i++) - nr_pages += kvm->memslots->memslots[i].npages; + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots); +for (i =0 ; i < slots->nmemslots; i++) + nr_pages += slots->memslots[i].npages; 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);return nr_mmu_pages; }@@ -3241,10 +3242,12 @@ static void audit_mappings(struct kvm_vcstatic int count_rmaps (struct kvm_vcpu *vcpu) {int nmaps =0 ; -int i, j, k; +int i, j, k, idx; + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots);for (i =0 ; i <KVM_MEMORY_SLOTS ; ++i) { -struct kvm_memory_slot *m = &vcpu->kvm->memslots->memslots[i]; +struct kvm_memory_slot *m = &slots->memslots[i];struct kvm_rmap_desc *d;for (j =0 ; j < m->npages; ++j) {@@ -3267,6 +3270,7 @@ static int count_rmaps(struct kvm_vcpu * } } } +srcu_read_unlock (&kvm->srcu, idx);return nmaps; }Index:
kvm-slotslock/arch/x86/kvm/vmx.c
===================================================================arch/x86/kvm/vmx.c +11
-2@@ -24,6 +24,7 @@#include <linux/mm.h> #include <linux/highmem.h> #include <linux/sched.h> +#include <linux/srcu.h> #include <linux/moduleparam.h> #include <linux/ftrace_event.h> #include "kvm_cache_regs.h" @@ -1465,10 +1466,18 @@ static void enter_pmode(struct kvm_vcpustatic gva_t rmode_tss_base (struct kvm *kvm) {if (!kvm->arch.tss_addr) { -gfn_t base_gfn = kvm->memslots->memslots[0 ].base_gfn + - kvm->memslots->memslots[0 ].npages -3 ; +struct kvm_memslots *slots; +gfn_t base_gfn; +int idx; + + idx =srcu_read_lock (&kvm->srcu); + slots =rcu_dereference (kvm->memslots); + base_gfn = slots->memslots[0 ].base_gfn + + slots->memslots[0 ].npages -3 ; +srcu_read_unlock (&kvm->srcu, idx);return base_gfn <<PAGE_SHIFT ; } +return kvm->arch.tss_addr; }Index:
kvm-slotslock/include/linux/kvm_host.h
===================================================================include/linux/kvm_host.h +1
-0@@ -163,6 +163,7 @@ struct kvm {struct rw_semaphore slots_lock;struct mm_struct *mm;/* userspace tied to this vm */ struct kvm_memslots *memslots; +struct srcu_struct srcu;#ifdef CONFIG_KVM_APIC_ARCHITECTURE u32 bsp_vcpu_id;struct kvm_vcpu *bsp_vcpu;Index:
kvm-slotslock/virt/kvm/assigned-dev.c
===================================================================virt/kvm/assigned-dev.c +4
-4@@ -504,11 +504,11 @@ out:static int kvm_vm_ioctl_assign_device (struct kvm *kvm,struct kvm_assigned_pci_dev *assigned_dev) { -int r =0 ; +int r =0 , idx;struct kvm_assigned_dev_kernel *match;struct pci_dev *dev; -down_read (&kvm->slots_lock); + idx =srcu_read_lock (&kvm->srcu);mutex_lock (&kvm->lock); match = kvm_find_assigned_dev(&kvm->arch.assigned_dev_head,@@ -574,7 +574,7 @@ static int kvm_vm_ioctl_assign_device(st out:mutex_unlock (&kvm->lock); -up_read (&kvm->slots_lock); +srcu_read_unlock (&kvm->srcu, idx);return r; out_list_del:list_del (&match->list);@@ -586,7 +586,7 @@ out_put: out_free:kfree (match);mutex_unlock (&kvm->lock); -up_read (&kvm->slots_lock); +srcu_read_unlock (&kvm->srcu, idx);return r; }Index:
kvm-slotslock/virt/kvm/iommu.c
===================================================================virt/kvm/iommu.c +2
-3@@ -78,7 +78,7 @@ static int kvm_iommu_map_memslots(structint i, r =0 ;struct kvm_memslots *slots; - slots = kvm->memslots; + slots =rcu_dereference (kvm->memslots);for (i =0 ; i < slots->nmemslots; i++) { r =kvm_iommu_map_pages (kvm, &slots->memslots[i]);@@ -214,7 +214,7 @@ static int kvm_iommu_unmap_memslots(struint i;struct kvm_memslots *slots; - slots = kvm->memslots; + slots =rcu_dereference (kvm->memslots);for (i =0 ; i < slots->nmemslots; i++) { kvm_iommu_put_pages(kvm, slots->memslots[i].base_gfn, --To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch 07/10] KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update
Quoted 22 lines · Fernando Carrijo
Resending with Cc: added
Quoted 6 lines · Marcelo Tosatti
virt/kvm/kvm_main.c +1
-1- kvm_arch_flush_shadow(kvm); + flush_shadow = 1; }#else /* not defined CONFIG_S390 */ new.user_alloc = user_alloc;virt/kvm/kvm_main.c skip_lpage: · line 649
Quoted 9 lines
virt/kvm/kvm_main.c +5
-1@@ -641,34 +642,69 @@ skip_lpage: new.userspace_addr = mem->userspace_addr;#endif /* not defined CONFIG_S390 */ -if (!npages) +if (!npages) { + slots =kzalloc (sizeof (struct kvm_memslots ),GFP_KERNEL ); +if (!slots) + goto out_free; +memcpy (slots, kvm->memslots,sizeof (struct kvm_memslots ));Nothing wrong with the above line, but it makes me think if
Quoted 3 lines
*slots = *kvm->memslots;would save us the function call overhead
Perhaps. But this is a slow path anyway, so it does not matter much.
Thanks for the review.
Re: [patch 07/10] KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update
In reply to Marcelo TosattiQuoted 8 lines · Marcelo Tosatti
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_hvalocking.Signed-off-by Marcelo Tosatti <[email protected]>
<snip>
Quoted 52 lines
virt/kvm/kvm_main.c +39
-6-if (!npages) +if (!npages) { + slots =kzalloc (sizeof (struct kvm_memslots ),GFP_KERNEL ); +if (!slots) + goto out_free; +memcpy (slots, kvm->memslots,sizeof (struct kvm_memslots )); +if (mem->slot >= slots->nmemslots) + slots->nmemslots = mem->slot +1 ; + slots->memslots[mem->slot].flags |=KVM_MEMSLOT_INVALID ; + + old_memslots = kvm->memslots; +rcu_assign_pointer (kvm->memslots, slots); +synchronize_srcu (&kvm->srcu); +/* 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); +kfree (old_memslots); + } r =kvm_arch_prepare_memory_region (kvm, &new, old, user_alloc);if (r) goto out_free; -spin_lock (&kvm->mmu_lock); -if (mem->slot >= kvm->memslots->nmemslots) - kvm->memslots->nmemslots = mem->slot +1 ; +#ifdef CONFIG_DMAR +/* map the pages in iommu page table */ +if (npages) + r =kvm_iommu_map_pages (kvm, &new); +if (r) + goto out_free; +#endif - *memslot = new; -spin_unlock (&kvm->mmu_lock); + slots =kzalloc (sizeof (struct kvm_memslots ),GFP_KERNEL ); +if (!slots) + goto out_free; +memcpy (slots, kvm->memslots,sizeof (struct kvm_memslots )); +if (mem->slot >= slots->nmemslots) + slots->nmemslots = mem->slot +1 ; + +/* actual memory is freed via old in kvm_free_physmem_slot below */ +if (!npages) { + new.rmap =NULL ; + new.dirty_bitmap =NULL ;
virt/kvm/kvm_main.c skip_lpage: · line 700
Quoted 10 lines
virt/kvm/kvm_main.c +8
-0+for (i =0 ; i <KVM_NR_PAGE_SIZES -1 ; ++i) + new.lpage_info[i] =NULL ; + } + + slots->memslots[mem->slot] = new; + old_memslots = kvm->memslots; +rcu_assign_pointer (kvm->memslots, slots); +synchronize_srcu (&kvm->srcu);kvm_arch_commit_memory_region (kvm, mem, old, user_alloc);
Paul,
There is a scenario where this path, which updates KVM memory slots, is called relatively often.
Each synchronize_srcu() call takes about 10ms (avg 3ms per synchronize_sched call), so this is hurting us.
Is this expected? Is there any possibility for synchronize_srcu() optimization?
There are other sides we can work on, such as reducing the memory slot updates, but i'm wondering what can be done regarding SRCU itself.
TIA
Re: [patch 07/10] KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update
Quoted 80 lines · Marcelo Tosatti
Quoted 8 lines · Marcelo Tosatti
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_hvalocking.Signed-off-by Marcelo Tosatti <[email protected]><snip>
Quoted 52 lines
virt/kvm/kvm_main.c +39
-6-if (!npages) +if (!npages) { + slots =kzalloc (sizeof (struct kvm_memslots ),GFP_KERNEL ); +if (!slots) + goto out_free; +memcpy (slots, kvm->memslots,sizeof (struct kvm_memslots )); +if (mem->slot >= slots->nmemslots) + slots->nmemslots = mem->slot +1 ; + slots->memslots[mem->slot].flags |=KVM_MEMSLOT_INVALID ; + + old_memslots = kvm->memslots; +rcu_assign_pointer (kvm->memslots, slots); +synchronize_srcu (&kvm->srcu); +/* 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); +kfree (old_memslots); + } r =kvm_arch_prepare_memory_region (kvm, &new, old, user_alloc);if (r) goto out_free; -spin_lock (&kvm->mmu_lock); -if (mem->slot >= kvm->memslots->nmemslots) - kvm->memslots->nmemslots = mem->slot +1 ; +#ifdef CONFIG_DMAR +/* map the pages in iommu page table */ +if (npages) + r =kvm_iommu_map_pages (kvm, &new); +if (r) + goto out_free; +#endif - *memslot = new; -spin_unlock (&kvm->mmu_lock); + slots =kzalloc (sizeof (struct kvm_memslots ),GFP_KERNEL ); +if (!slots) + goto out_free; +memcpy (slots, kvm->memslots,sizeof (struct kvm_memslots )); +if (mem->slot >= slots->nmemslots) + slots->nmemslots = mem->slot +1 ; + +/* actual memory is freed via old in kvm_free_physmem_slot below */ +if (!npages) { + new.rmap =NULL ; + new.dirty_bitmap =NULL ;virt/kvm/kvm_main.c skip_lpage: · line 700
Quoted 10 lines
virt/kvm/kvm_main.c +8
-0+for (i =0 ; i <KVM_NR_PAGE_SIZES -1 ; ++i) + new.lpage_info[i] =NULL ; + } + + slots->memslots[mem->slot] = new; + old_memslots = kvm->memslots; +rcu_assign_pointer (kvm->memslots, slots); +synchronize_srcu (&kvm->srcu);kvm_arch_commit_memory_region (kvm, mem, old, user_alloc);Paul,
There is a scenario where this path, which updates KVM memory slots, is called relatively often.
Quoted 8 lines
Each
synchronize_srcu()call takes about 10ms (avg 3ms persynchronize_schedcall), so this is hurting us.Is this expected? Is there any possibility for
synchronize_srcu()optimization?There are other sides we can work on, such as reducing the memory slot updates, but i'm wondering what can be done regarding SRCU itself.
This is expected behavior, but there is a possible fix currently in mainline (Linus's git tree). The idea would be to create a synchronize_srcu_expedited(), which starts with synchronize_srcu(), and replaces the synchronize_sched() calls with synchronize_sched_expedited().
This could potentially reduce the overall synchronize_srcu() latency to well under a microsecond. The price to be paid is that each instance of synchronize_sched_expedited() IPIs all the online CPUs, and awakens the migration thread on each.
Would this approach likely work for you?
Thanx, Paul
Re: [patch 07/10] KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update
Quoted 15 lines · Paul E. McKenney
Quoted 10 lines
Each
synchronize_srcu()call takes about 10ms (avg 3ms persynchronize_schedcall), so this is hurting us.Is this expected? Is there any possibility for
synchronize_srcu()optimization?There are other sides we can work on, such as reducing the memory slot updates, but i'm wondering what can be done regarding SRCU itself.
This is expected behavior, but there is a possible fix currently in mainline (Linus's git tree). The idea would be to create a
synchronize_srcu_expedited(), which starts withsynchronize_srcu(), and replaces thesynchronize_sched()calls withsynchronize_sched_expedited().
Quoted 7 lines
This could potentially reduce the overall
synchronize_srcu()latency to well under a microsecond. The price to be paid is that each instance ofsynchronize_sched_expedited()IPIs all the online CPUs, and awakens the migration thread on each.Would this approach likely work for you?
It's perfect.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Re: [patch 07/10] KVM: introduce kvm->srcu and convert kvm_set_memory_region to SRCU update
In reply to Paul E. McKenneyQuoted 19 lines · Paul E. McKenney
Quoted 5 lines
Paul,
There is a scenario where this path, which updates KVM memory slots, is called relatively often.
Quoted 8 lines
Each
synchronize_srcu()call takes about 10ms (avg 3ms persynchronize_schedcall), so this is hurting us.Is this expected? Is there any possibility for
synchronize_srcu()optimization?There are other sides we can work on, such as reducing the memory slot updates, but i'm wondering what can be done regarding SRCU itself.
This is expected behavior, but there is a possible fix currently in mainline (Linus's git tree). The idea would be to create a
synchronize_srcu_expedited(), which starts withsynchronize_srcu(), and replaces thesynchronize_sched()calls withsynchronize_sched_expedited().
Quoted 6 lines
This could potentially reduce the overall
synchronize_srcu()latency to well under a microsecond. The price to be paid is that each instance ofsynchronize_sched_expedited()IPIs all the online CPUs, and awakens the migration thread on each.Would this approach likely work for you?
Hum, this path can be triggered by a guest, so IPI'ing all online CPUs is not a happy thought.
PATCH 8/10 KVM: x86: switch kvm_set_memory_alias to SRCU update
In reply to Marcelo TosattiUsing a similar two-step procedure as for memslots.
arch/x86/include/asm/kvm_host.h +3 -0
@@ -373,10 +373,13 @@ struct kvm_vcpu_arch {u64 *mce_banks; }; +#define KVM_ALIAS_INVALID 1UL +struct kvm_mem_alias {gfn_t base_gfn;unsigned long npages;gfn_t target_gfn; +unsigned long flags; };struct kvm_mem_aliases {
arch/x86/kvm/mmu.c +7 -7
@@ -424,7 +424,7 @@ static void account_shadowed(struct kvmint *write_count;int i; - gfn =unalias_gfn (kvm, gfn); + gfn =unalias_gfn (kvm, gfn, false); slot =gfn_to_memslot_unaliased (kvm, gfn); for (i = PT_DIRECTORY_LEVEL;
@@ -440,7 +440,7 @@ static void unaccount_shadowed(struct kvint *write_count;int i; - gfn =unalias_gfn (kvm, gfn); + gfn =unalias_gfn (kvm, gfn, false);for (i =PT_DIRECTORY_LEVEL ; i <PT_PAGE_TABLE_LEVEL +KVM_NR_PAGE_SIZES ; ++i) { slot =gfn_to_memslot_unaliased (kvm, gfn);
@@ -457,7 +457,7 @@ static int has_wrprotected_page(struct kstruct kvm_memory_slot *slot;int *largepage_idx; - gfn =unalias_gfn (kvm, gfn); + gfn =unalias_gfn (kvm, gfn, false); slot =gfn_to_memslot_unaliased (kvm, gfn);if (slot) { largepage_idx =slot_largepage_idx (gfn, slot, level);
@@ -565,7 +565,7 @@ static int rmap_add(struct kvm_vcpu *vcpif (!is_rmap_spte (*spte))return count; - gfn =unalias_gfn (vcpu->kvm, gfn); + gfn =unalias_gfn (vcpu->kvm, gfn, false); sp =page_header (__pa (spte)); sp->gfns[spte - sp->spt] = gfn; rmapp =gfn_to_rmap (vcpu->kvm, gfn, sp->role.level);
@@ -703,7 +703,7 @@ static int rmap_write_protect(struct kvmu64 *spte;int i, write_protected =0 ; - gfn =unalias_gfn (kvm, gfn); + gfn =unalias_gfn (kvm, gfn, false); rmapp =gfn_to_rmap (kvm, gfn,PT_PAGE_TABLE_LEVEL ); spte =rmap_next (kvm, rmapp,NULL );
@@ -836,7 +836,7 @@ static void rmap_recycle(struct kvm_vcpu sp =page_header (__pa (spte)); - gfn =unalias_gfn (vcpu->kvm, gfn); + gfn =unalias_gfn (vcpu->kvm, gfn, false); rmapp =gfn_to_rmap (vcpu->kvm, gfn, sp->role.level);kvm_unmap_rmapp (vcpu->kvm, rmapp);
@@ -3358,7 +3358,7 @@ static void audit_write_protection(strucif (sp->unsync)continue ; - gfn =unalias_gfn (vcpu->kvm, sp->gfn); + gfn =unalias_gfn (vcpu->kvm, sp->gfn, false); slot =gfn_to_memslot_unaliased (vcpu->kvm, sp->gfn); rmapp = &slot->rmap[gfn - slot->base_gfn];
arch/x86/kvm/x86.c +32 -7
@@ -37,6 +37,7 @@#include <linux/iommu.h> #include <linux/intel-iommu.h> #include <linux/cpufreq.h> +#include <linux/srcu.h> #include <trace/events/kvm.h> #undef TRACE_INCLUDE_FILE#define CREATE_TRACE_POINTS
@@ -1957,14 +1958,18 @@ static int kvm_vm_ioctl_get_nr_mmu_pagesreturn kvm->arch.n_alloc_mmu_pages; } -gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn) +gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn,bool instantiation) {int i;struct kvm_mem_alias *alias; -struct kvm_mem_aliases *aliases = kvm->arch.aliases; +struct kvm_mem_aliases *aliases; + + aliases =rcu_dereference (kvm->arch.aliases);for (i =0 ; i < aliases->naliases; ++i) { alias = &aliases->aliases[i]; +if (instantiation && (alias->flags &KVM_ALIAS_INVALID )) +continue ;if (gfn >= alias->base_gfn && gfn < alias->base_gfn + alias->npages)return alias->target_gfn + gfn - alias->base_gfn;
@@ -1982,7 +1987,7 @@ static int kvm_vm_ioctl_set_memory_alias {int r, n;struct kvm_mem_alias *p; -struct kvm_mem_aliases *aliases; +struct kvm_mem_aliases *aliases, *old_aliases; r = -EINVAL ;/* General sanity checks */
@@ -1999,23 +2004,43 @@ static int kvm_vm_ioctl_set_memory_alias < alias->target_phys_addr) goto out; + r = -ENOMEM ; + aliases =kzalloc (sizeof (struct kvm_mem_aliases ),GFP_KERNEL ); +if (!aliases) + goto out; +down_write (&kvm->slots_lock); -spin_lock (&kvm->mmu_lock); - aliases = kvm->arch.aliases; +/* invalidate any gfn reference in case of deletion/shrinking */ +memcpy (aliases, kvm->arch.aliases,sizeof (struct kvm_mem_aliases )); + aliases->aliases[alias->slot].flags |=KVM_ALIAS_INVALID ; + old_aliases = kvm->arch.aliases; +rcu_assign_pointer (kvm->arch.aliases, aliases); +synchronize_srcu (&kvm->srcu); +kvm_mmu_zap_all (kvm); +kfree (old_aliases); + + r = -ENOMEM ; + aliases =kzalloc (sizeof (struct kvm_mem_aliases ),GFP_KERNEL ); +if (!aliases) + goto out; +memcpy (aliases, kvm->arch.aliases,sizeof (struct kvm_mem_aliases )); p = &aliases->aliases[alias->slot]; p->base_gfn = alias->guest_phys_addr >>PAGE_SHIFT ; p->npages = alias->memory_size >>PAGE_SHIFT ; p->target_gfn = alias->target_phys_addr >>PAGE_SHIFT ; + p->flags &= ~(KVM_ALIAS_INVALID );for (n =KVM_ALIAS_SLOTS ; n >0 ; --n)if (aliases->aliases[n -1 ].npages)break ; aliases->naliases = n; -spin_unlock (&kvm->mmu_lock); -kvm_mmu_zap_all (kvm); + old_aliases = kvm->arch.aliases; +rcu_assign_pointer (kvm->arch.aliases, aliases); +synchronize_srcu (&kvm->srcu); +kfree (old_aliases);up_write (&kvm->slots_lock);
include/linux/kvm_host.h +1 -1
@@ -265,7 +265,7 @@ void kvm_arch_commit_memory_region(struc int user_alloc);void kvm_disable_largepages (void );void kvm_arch_flush_shadow (struct kvm *kvm); -gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn); +gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn,bool instantiation);struct page *gfn_to_page (struct kvm *kvm,gfn_t gfn);unsigned long gfn_to_hva (struct kvm *kvm,gfn_t gfn);void kvm_release_page_clean (struct page *page);
virt/kvm/kvm_main.c +4 -4
@@ -819,7 +819,7 @@ EXPORT_SYMBOL_GPL(gfn_to_memslot_unaliasstruct kvm_memory_slot *gfn_to_memslot (struct kvm *kvm,gfn_t gfn) { - gfn =unalias_gfn (kvm, gfn); + gfn =unalias_gfn (kvm, gfn, false);return gfn_to_memslot_unaliased (kvm, gfn); }
@@ -828,7 +828,7 @@ int kvm_is_visible_gfn(struct kvm *kvm,int i;struct kvm_memslots *slots =rcu_dereference (kvm->memslots); - gfn =unalias_gfn (kvm, gfn); + gfn =unalias_gfn (kvm, gfn, true);for (i =0 ; i <KVM_MEMORY_SLOTS ; ++i) {struct kvm_memory_slot *memslot = &slots->memslots[i];
@@ -847,7 +847,7 @@ unsigned long gfn_to_hva(struct kvm *kvm {struct kvm_memory_slot *slot; - gfn =unalias_gfn (kvm, gfn); + gfn =unalias_gfn (kvm, gfn, true); slot =gfn_to_memslot_unaliased (kvm, gfn);if (!slot || slot->flags &KVM_MEMSLOT_INVALID )return bad_hva ();
@@ -1117,7 +1117,7 @@ void mark_page_dirty(struct kvm *kvm, gf {struct kvm_memory_slot *memslot; - gfn =unalias_gfn (kvm, gfn); + gfn =unalias_gfn (kvm, gfn, false); memslot =gfn_to_memslot_unaliased (kvm, gfn);if (memslot && memslot->dirty_bitmap) {unsigned long rel_gfn = gfn - memslot->base_gfn;
arch/ia64/kvm/kvm-ia64.c +1 -1
@@ -1946,7 +1946,7 @@ int kvm_cpu_has_pending_timer(struct kvmreturn vcpu->arch.timer_fired; } -gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn) +gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn,bool instantiation) {return gfn; }
arch/powerpc/kvm/powerpc.c +1 -1
@@ -34,7 +34,7 @@#define CREATE_TRACE_POINTS #include "trace.h" -gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn) +gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn,bool instantiation) {return gfn; }
arch/s390/kvm/kvm-s390.c +1 -1
@@ -730,7 +730,7 @@ void kvm_arch_flush_shadow(struct kvm *k { } -gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn) +gfn_t unalias_gfn (struct kvm *kvm,gfn_t gfn,bool instantiation) {return gfn; }
Re: [patch 08/10] KVM: x86: switch kvm_set_memory_alias to SRCU update
Review
Quoted 7 lines · Marcelo Tosatti
Using a similar two-step procedure as for memslots.
(quoted diff) +1
-1- gfn = unalias_gfn(kvm, gfn); + gfn = unalias_gfn(kvm, gfn, false);
To improve readability suggest two names, maybe unalias_gfn() and unalias_gfn_instantiation(). boolean parameters are hard to read at the call site.
Also we've discussed in the past converting aliases to private slots (and also echo aliases >> feature-removal-schedule.txt); maybe that is an easier way of verifying everything still works.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
PATCH 9/10 KVM: convert io_bus to SRCU
In reply to Marcelo Tosattiarch/x86/kvm/i8254.c +3 -3
@@ -640,13 +640,13 @@ struct kvm_pit *kvm_create_pit(struct kvkvm_register_irq_mask_notifier (kvm,0 , &pit->mask_notifier);kvm_iodevice_init (&pit->dev, &pit_dev_ops); - ret =__kvm_io_bus_register_dev (&kvm->pio_bus, &pit->dev); + ret =kvm_io_bus_register_dev (kvm,KVM_PIO_BUS , &pit->dev);if (ret <0 ) goto fail;if (flags &KVM_PIT_SPEAKER_DUMMY ) {kvm_iodevice_init (&pit->speaker_dev, &speaker_dev_ops); - ret =__kvm_io_bus_register_dev (&kvm->pio_bus, + ret =kvm_io_bus_register_dev (kvm,KVM_PIO_BUS , &pit->speaker_dev);if (ret <0 ) goto fail_unregister;
@@ -655,7 +655,7 @@ struct kvm_pit *kvm_create_pit(struct kvreturn pit; fail_unregister: -__kvm_io_bus_unregister_dev (&kvm->pio_bus, &pit->dev); +kvm_io_bus_unregister_dev (kvm,KVM_PIO_BUS , &pit->dev); fail:if (pit->irq_source_id >=0 )
arch/x86/kvm/i8259.c +3 -1
@@ -533,7 +533,9 @@ struct kvm_pic *kvm_create_pic(struct kv* Initialize PIO device */ kvm_iodevice_init (&s->dev, &picdev_ops); - ret =kvm_io_bus_register_dev (kvm, &kvm->pio_bus, &s->dev); +down_write (&kvm->slots_lock); + ret =kvm_io_bus_register_dev (kvm,KVM_PIO_BUS , &s->dev); +up_write (&kvm->slots_lock);if (ret <0 ) {kfree (s);return NULL ;
include/linux/kvm_host.h +13 -14
@@ -58,20 +58,20 @@ struct kvm_io_bus {struct kvm_io_device *devs[NR_IOBUS_DEVS ]; }; -void kvm_io_bus_init (struct kvm_io_bus *bus); -void kvm_io_bus_destroy (struct kvm_io_bus *bus); -int kvm_io_bus_write (struct kvm_io_bus *bus,gpa_t addr,int len, -const void *val); -int kvm_io_bus_read (struct kvm_io_bus *bus,gpa_t addr,int len, +enum kvm_bus { +KVM_MMIO_BUS , +KVM_PIO_BUS , +KVM_NR_BUSES +}; + +int kvm_io_bus_write (struct kvm *kvm,enum kvm_bus bus_idx,gpa_t addr, +int len,const void *val); +int kvm_io_bus_read (struct kvm *kvm,enum kvm_bus bus_idx,gpa_t addr,int len,void *val); -int __kvm_io_bus_register_dev (struct kvm_io_bus *bus, -struct kvm_io_device *dev); -int kvm_io_bus_register_dev (struct kvm *kvm,struct kvm_io_bus *bus, +int kvm_io_bus_register_dev (struct kvm *kvm,enum kvm_bus bus_idx,struct kvm_io_device *dev); -void __kvm_io_bus_unregister_dev (struct kvm_io_bus *bus, -struct kvm_io_device *dev); -void kvm_io_bus_unregister_dev (struct kvm *kvm,struct kvm_io_bus *bus, -struct kvm_io_device *dev); +int kvm_io_bus_unregister_dev (struct kvm *kvm,enum kvm_bus bus_idx, +struct kvm_io_device *dev);struct kvm_vcpu {struct kvm *kvm;
@@ -172,8 +172,7 @@ struct kvm {atomic_t online_vcpus;struct list_head vm_list;struct mutex lock; -struct kvm_io_bus mmio_bus; -struct kvm_io_bus pio_bus; +struct kvm_io_bus *buses[KVM_NR_BUSES ]; #ifdef CONFIG_HAVE_KVM_EVENTFD struct { spinlock_t lock;
virt/kvm/eventfd.c +4 -4
@@ -451,7 +451,7 @@ static int kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) { int pio = args->flags & KVM_IOEVENTFD_FLAG_PIO; -struct kvm_io_bus *bus = pio ? &kvm->pio_bus : &kvm->mmio_bus; +enum kvm_bus bus_idx = pio ?KVM_PIO_BUS :KVM_MMIO_BUS ;struct _ioeventfd *p;struct eventfd_ctx *eventfd;int ret;
@@ -506,7 +506,7 @@ kvm_assign_ioeventfd(struct kvm *kvm, stkvm_iodevice_init (&p->dev, &ioeventfd_ops); - ret =__kvm_io_bus_register_dev (bus, &p->dev); + ret =kvm_io_bus_register_dev (kvm, bus_idx, &p->dev);if (ret <0 ) goto unlock_fail;
@@ -530,7 +530,7 @@ static int kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) { int pio = args->flags & KVM_IOEVENTFD_FLAG_PIO; -struct kvm_io_bus *bus = pio ? &kvm->pio_bus : &kvm->mmio_bus; +enum kvm_bus bus_idx = pio ?KVM_PIO_BUS :KVM_MMIO_BUS ;struct _ioeventfd *p, *tmp;struct eventfd_ctx *eventfd;int ret = -ENOENT ;
@@ -553,7 +553,7 @@ kvm_deassign_ioeventfd(struct kvm *kvm,if (!p->wildcard && p->datamatch != args->datamatch)continue ; -__kvm_io_bus_unregister_dev (bus, &p->dev); +kvm_io_bus_unregister_dev (kvm, bus_idx, &p->dev);ioeventfd_release (p); ret =0 ;break ;
virt/kvm/ioapic.c +3 -1
@@ -372,7 +372,9 @@ int kvm_ioapic_init(struct kvm *kvm)kvm_ioapic_reset (ioapic);kvm_iodevice_init (&ioapic->dev, &ioapic_mmio_ops); ioapic->kvm = kvm; - ret =kvm_io_bus_register_dev (kvm, &kvm->mmio_bus, &ioapic->dev); +down_write (&kvm->slots_lock); + ret =kvm_io_bus_register_dev (kvm,KVM_MMIO_BUS , &ioapic->dev); +up_write (&kvm->slots_lock);if (ret <0 )kfree (ioapic);
virt/kvm/kvm_main.c +63 -43
197 lines not on this page · raw message
virt/kvm/coalesced_mmio.c +3 -1
@@ -102,7 +102,9 @@ int kvm_coalesced_mmio_init(struct kvm * dev->kvm = kvm; kvm->coalesced_mmio_dev = dev; - ret =kvm_io_bus_register_dev (kvm, &kvm->mmio_bus, &dev->dev); +down_write (&kvm->slots_lock); + ret =kvm_io_bus_register_dev (kvm,KVM_MMIO_BUS , &dev->dev); +up_write (&kvm->slots_lock);if (ret <0 )kfree (dev);
arch/x86/kvm/x86.c +7 -6
@@ -2489,7 +2489,7 @@ static int vcpu_mmio_write(struct kvm_vc !kvm_iodevice_write(&vcpu->arch.apic->dev, addr, len, v))return 0 ; -return kvm_io_bus_write (&vcpu->kvm->mmio_bus, addr, len, v); +return kvm_io_bus_write (vcpu->kvm,KVM_MMIO_BUS , addr, len, v); }static int vcpu_mmio_read (struct kvm_vcpu *vcpu,gpa_t addr,int len,void *v)
@@ -2498,7 +2498,7 @@ static int vcpu_mmio_read(struct kvm_vcp !kvm_iodevice_read(&vcpu->arch.apic->dev, addr, len, v))return 0 ; -return kvm_io_bus_read (&vcpu->kvm->mmio_bus, addr, len, v); +return kvm_io_bus_read (vcpu->kvm,KVM_MMIO_BUS , addr, len, v); } static int kvm_read_guest_virt(gva_t addr, void *val, unsigned int bytes,
@@ -2983,11 +2983,12 @@ static int kernel_pio(struct kvm_vcpu *vint r;if (vcpu->arch.pio.in) - r =kvm_io_bus_read (&vcpu->kvm->pio_bus, vcpu->arch.pio.port, + r =kvm_io_bus_read (vcpu->kvm,KVM_PIO_BUS , vcpu->arch.pio.port, vcpu->arch.pio.size, pd);else - r =kvm_io_bus_write (&vcpu->kvm->pio_bus, vcpu->arch.pio.port, - vcpu->arch.pio.size, pd); + r =kvm_io_bus_write (vcpu->kvm,KVM_PIO_BUS , + vcpu->arch.pio.port, vcpu->arch.pio.size, + pd);return r; }
@@ -2998,7 +2999,7 @@ static int pio_string_write(struct kvm_vint i, r =0 ;for (i =0 ; i < io->cur_count; i++) { -if (kvm_io_bus_write (&vcpu->kvm->pio_bus, +if (kvm_io_bus_write (vcpu->kvm,KVM_PIO_BUS , io->port, io->size, pd)) { r = -EOPNOTSUPP ;break ;
arch/ia64/kvm/kvm-ia64.c +2 -2
@@ -241,10 +241,10 @@ static int handle_mmio(struct kvm_vcpu *return 0 ; mmio:if (p->dir) - r =kvm_io_bus_read (&vcpu->kvm->mmio_bus, p->addr, + r =kvm_io_bus_read (vcpu->kvm,KVM_MMIO_BUS , p->addr, p->size, &p->data);else - r =kvm_io_bus_write (&vcpu->kvm->mmio_bus, p->addr, + r =kvm_io_bus_write (vcpu->kvm,KVM_MMIO_BUS , p->addr, p->size, &p->data);if (r)printk (KERN_ERR "kvm: No iodevice found! addr:%lx\n" , p->addr);
PATCH 10/10 KVM: switch vcpu context to use SRCU
In reply to Marcelo Tosattiarch/x86/kvm/x86.c +24 -19
@@ -1140,15 +1140,15 @@ static int __msr_io(struct kvm_vcpu *vcpint (*do_msr)(struct kvm_vcpu *vcpu, unsigned index, u64 *data)) { -int i; +int i, idx;vcpu_load (vcpu); -down_read (&vcpu->kvm->slots_lock); + idx =srcu_read_lock (&vcpu->kvm->srcu);for (i =0 ; i < msrs->nmsrs; ++i)if (do_msr (vcpu, entries[i].index, &entries[i].data))break ; -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&vcpu->kvm->srcu, idx);vcpu_put (vcpu);
@@ -3531,16 +3531,17 @@ static void vapic_enter(struct kvm_vcpustatic void vapic_exit (struct kvm_vcpu *vcpu) {struct kvm_lapic *apic = vcpu->arch.apic; +int idx;if (!apic || !apic->vapic_addr)return ; -down_read (&vcpu->kvm->slots_lock); + idx =srcu_read_lock (&vcpu->kvm->srcu);kvm_release_page_dirty (apic->vapic_page);spin_lock (&vcpu->kvm->mmu_lock);mark_page_dirty (vcpu->kvm, apic->vapic_addr >>PAGE_SHIFT );spin_unlock (&vcpu->kvm->mmu_lock); -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&vcpu->kvm->srcu, idx); }static void update_cr8_intercept (struct kvm_vcpu *vcpu)
@@ -3669,7 +3670,7 @@ static int vcpu_enter_guest(struct kvm_vkvm_lapic_sync_to_vapic (vcpu); } -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&vcpu->kvm->srcu, vcpu->srcu_idx);kvm_guest_enter ();
@@ -3710,7 +3711,7 @@ static int vcpu_enter_guest(struct kvm_vpreempt_enable (); -down_read (&vcpu->kvm->slots_lock); + vcpu->srcu_idx =srcu_read_lock (&vcpu->kvm->srcu);/* * Profile KVM exit RIPs:
@@ -3732,6 +3733,7 @@ out:static int __vcpu_run (struct kvm_vcpu *vcpu) {int r; +struct kvm *kvm = vcpu->kvm;if (unlikely (vcpu->arch.mp_state ==KVM_MP_STATE_SIPI_RECEIVED )) { pr_debug("vcpu %d received sipi with vector # %x\n",
@@ -3743,7 +3745,7 @@ static int __vcpu_run(struct kvm_vcpu *v vcpu->arch.mp_state =KVM_MP_STATE_RUNNABLE ; } -down_read (&vcpu->kvm->slots_lock); + vcpu->srcu_idx =srcu_read_lock (&kvm->srcu);vapic_enter (vcpu); r =1 ;
@@ -3751,9 +3753,9 @@ static int __vcpu_run(struct kvm_vcpu *vif (vcpu->arch.mp_state ==KVM_MP_STATE_RUNNABLE ) r =vcpu_enter_guest (vcpu);else { -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&kvm->srcu, vcpu->srcu_idx);kvm_vcpu_block (vcpu); -down_read (&vcpu->kvm->slots_lock); + vcpu->srcu_idx =srcu_read_lock (&kvm->srcu);if (test_and_clear_bit (KVM_REQ_UNHALT , &vcpu->requests)) {switch (vcpu->arch.mp_state) {
@@ -3788,13 +3790,13 @@ static int __vcpu_run(struct kvm_vcpu *v ++vcpu->stat.signal_exits; }if (need_resched ()) { -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&kvm->srcu, vcpu->srcu_idx);kvm_resched (vcpu); -down_read (&vcpu->kvm->slots_lock); + vcpu->srcu_idx =srcu_read_lock (&kvm->srcu); } } -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&kvm->srcu, vcpu->srcu_idx);post_kvm_run_save (vcpu);vapic_exit (vcpu);
@@ -3834,10 +3836,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_v vcpu->mmio_read_completed =1 ; vcpu->mmio_needed =0 ; -down_read (&vcpu->kvm->slots_lock); + vcpu->srcu_idx =srcu_read_lock (&vcpu->kvm->srcu); r =emulate_instruction (vcpu, vcpu->arch.mmio_fault_cr2,0 ,EMULTYPE_NO_DECODE ); -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&vcpu->kvm->srcu, vcpu->srcu_idx);if (r ==EMULATE_DO_MMIO ) {/* * Read-modify-write. Back to userspace.
@@ -4575,11 +4577,12 @@ int kvm_arch_vcpu_ioctl_translate(struct {unsigned long vaddr = tr->linear_address;gpa_t gpa; +int idx;vcpu_load (vcpu); -down_read (&vcpu->kvm->slots_lock); + idx =srcu_read_lock (&vcpu->kvm->srcu); gpa = vcpu->arch.mmu.gva_to_gpa (vcpu, vaddr); -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&vcpu->kvm->srcu, idx); tr->physical_address = gpa; tr->valid = gpa !=UNMAPPED_GVA ; tr->writeable =1 ;
@@ -4818,10 +4821,12 @@ fail:void kvm_arch_vcpu_uninit (struct kvm_vcpu *vcpu) { +int idx; +kvm_free_lapic (vcpu); -down_read (&vcpu->kvm->slots_lock); + idx =srcu_read_lock (&vcpu->kvm->srcu);kvm_mmu_destroy (vcpu); -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&vcpu->kvm->srcu, idx);free_page ((unsigned long )vcpu->arch.pio_data); }
include/linux/kvm_host.h +2 -0
@@ -84,6 +84,8 @@ struct kvm_vcpu {struct kvm_run *run;unsigned long requests;unsigned long guest_debug; +int srcu_idx; +int fpu_active;int guest_fpu_loaded;wait_queue_head_t wq;
arch/x86/kvm/mmu.c +3 -4
@@ -2883,10 +2883,9 @@ static int mmu_shrink(int nr_to_scan, gfspin_lock (&kvm_lock);list_for_each_entry (kvm, &vm_list, vm_list) { -int npages; +int npages, idx; -if (!down_read_trylock (&kvm->slots_lock)) -continue ; + idx =srcu_read_lock (&kvm->srcu);spin_lock (&kvm->mmu_lock); npages = kvm->arch.n_alloc_mmu_pages - kvm->arch.n_free_mmu_pages;
@@ -2899,7 +2898,7 @@ static int mmu_shrink(int nr_to_scan, gf nr_to_scan--;spin_unlock (&kvm->mmu_lock); -up_read (&kvm->slots_lock); +srcu_read_unlock (&kvm->srcu, idx); }if (kvm_freed)list_move_tail (&kvm_freed->vm_list, &vm_list);
arch/x86/kvm/vmx.c +3 -3
@@ -2429,10 +2429,10 @@ static int vmx_vcpu_reset(struct kvm_vcp {struct vcpu_vmx *vmx =to_vmx (vcpu);u64 msr; -int ret; +int ret, idx; vcpu->arch.regs_avail = ~((1 <<VCPU_REGS_RIP ) | (1 <<VCPU_REGS_RSP )); -down_read (&vcpu->kvm->slots_lock); + idx =srcu_read_lock (&vcpu->kvm->srcu);if (!init_rmode (vmx->vcpu.kvm)) { ret = -ENOMEM ; goto out;
@@ -2540,7 +2540,7 @@ static int vmx_vcpu_reset(struct kvm_vcp vmx->emulation_required =0 ; out: -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&vcpu->kvm->srcu, idx);return ret; }
arch/ia64/kvm/kvm-ia64.c +6 -9
@@ -636,12 +636,9 @@ static void kvm_vcpu_post_transition(strstatic int __vcpu_run (struct kvm_vcpu *vcpu,struct kvm_run *kvm_run) {union context *host_ctx, *guest_ctx; -int r; +int r, idx; -/* -* down_read() may sleep and return with interrupts enabled -*/ -down_read (&vcpu->kvm->slots_lock); + idx =srcu_read_lock (&vcpu->kvm->srcu); again:if (signal_pending (current)) {
@@ -663,7 +660,7 @@ again:if (r <0 ) goto vcpu_run_fail; -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&vcpu->kvm->srcu, idx);kvm_guest_enter ();/*
@@ -687,7 +684,7 @@ again:kvm_guest_exit ();preempt_enable (); -down_read (&vcpu->kvm->slots_lock); + idx =srcu_read_lock (&vcpu->kvm->srcu); r =kvm_handle_exit (kvm_run, vcpu);
@@ -697,10 +694,10 @@ again: } out: -up_read (&vcpu->kvm->slots_lock); +srcu_read_unlock (&vcpu->kvm->srcu, idx);if (r >0 ) {kvm_resched (vcpu); -down_read (&vcpu->kvm->slots_lock); + idx =srcu_read_lock (&vcpu->kvm->srcu); goto again; }
Re: [patch 10/10] KVM: switch vcpu context to use SRCU
Quoted 3 lines · Marcelo Tosatti
Signed-off-by Marcelo Tosatti<[email protected]>
I'd like an eleventh patch that converts slots_lock to a mutex; there should be no readers left, yes?
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
Quoted 5 lines · Marcelo Tosatti
Improves vmexit latency by ~= 13% on UP guest.
Still requires decent stress testing and careful review, but posting for early comments.
Okay, looks very nice. The only real problem I see is mmu_lock for mark_dirty(), the other comments are more or less cosmetic.
Please copy the kvm-arch lists and maintainers on the next round so they can review this as well.
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.