aboutsummaryrefslogtreecommitdiffstats
path: root/virt
diff options
context:
space:
mode:
authorDavid Matlack <dmatlack@google.com>2014-08-18 18:46:06 -0400
committerPaolo Bonzini <pbonzini@redhat.com>2014-09-03 04:03:41 -0400
commitee3d1570b58677885b4552bce8217fda7b226a68 (patch)
treebfc831cf5b242321553fbd6a702085a1916fd04c /virt
parent00f034a12fdd81210d58116326d92780aac5c238 (diff)
kvm: fix potentially corrupt mmio cache
vcpu exits and memslot mutations can run concurrently as long as the vcpu does not aquire the slots mutex. Thus it is theoretically possible for memslots to change underneath a vcpu that is handling an exit. If we increment the memslot generation number again after synchronize_srcu_expedited(), vcpus can safely cache memslot generation without maintaining a single rcu_dereference through an entire vm exit. And much of the x86/kvm code does not maintain a single rcu_dereference of the current memslots during each exit. We can prevent the following case: vcpu (CPU 0) | thread (CPU 1) --------------------------------------------+-------------------------- 1 vm exit | 2 srcu_read_unlock(&kvm->srcu) | 3 decide to cache something based on | old memslots | 4 | change memslots | (increments generation) 5 | synchronize_srcu(&kvm->srcu); 6 retrieve generation # from new memslots | 7 tag cache with new memslot generation | 8 srcu_read_unlock(&kvm->srcu) | ... | <action based on cache occurs even | though the caching decision was based | on the old memslots> | ... | <action *continues* to occur until next | memslot generation change, which may | be never> | | By incrementing the generation after synchronizing with kvm->srcu readers, we ensure that the generation retrieved in (6) will become invalid soon after (8). Keeping the existing increment is not strictly necessary, but we do keep it and just move it for consistency from update_memslots to install_new_memslots. It invalidates old cached MMIOs immediately, instead of having to wait for the end of synchronize_srcu_expedited, which makes the code more clearly correct in case CPU 1 is preempted right after synchronize_srcu() returns. To avoid halving the generation space in SPTEs, always presume that the low bit of the generation is zero when reconstructing a generation number out of an SPTE. This effectively disables MMIO caching in SPTEs during the call to synchronize_srcu_expedited. Using the low bit this way is somewhat like a seqcount---where the protected thing is a cache, and instead of retrying we can simply punt if we observe the low bit to be 1. Cc: stable@vger.kernel.org Signed-off-by: David Matlack <dmatlack@google.com> Reviewed-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com> Reviewed-by: David Matlack <dmatlack@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Diffstat (limited to 'virt')
-rw-r--r--virt/kvm/kvm_main.c23
1 files changed, 16 insertions, 7 deletions
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 0bfdb673db26..bb8641b5d83b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -95,8 +95,6 @@ static int hardware_enable_all(void);
95static void hardware_disable_all(void); 95static void hardware_disable_all(void);
96 96
97static void kvm_io_bus_destroy(struct kvm_io_bus *bus); 97static void kvm_io_bus_destroy(struct kvm_io_bus *bus);
98static void update_memslots(struct kvm_memslots *slots,
99 struct kvm_memory_slot *new, u64 last_generation);
100 98
101static void kvm_release_pfn_dirty(pfn_t pfn); 99static void kvm_release_pfn_dirty(pfn_t pfn);
102static void mark_page_dirty_in_slot(struct kvm *kvm, 100static void mark_page_dirty_in_slot(struct kvm *kvm,
@@ -695,8 +693,7 @@ static void sort_memslots(struct kvm_memslots *slots)
695} 693}
696 694
697static void update_memslots(struct kvm_memslots *slots, 695static void update_memslots(struct kvm_memslots *slots,
698 struct kvm_memory_slot *new, 696 struct kvm_memory_slot *new)
699 u64 last_generation)
700{ 697{
701 if (new) { 698 if (new) {
702 int id = new->id; 699 int id = new->id;
@@ -707,8 +704,6 @@ static void update_memslots(struct kvm_memslots *slots,
707 if (new->npages != npages) 704 if (new->npages != npages)
708 sort_memslots(slots); 705 sort_memslots(slots);
709 } 706 }
710
711 slots->generation = last_generation + 1;
712} 707}
713 708
714static int check_memory_region_flags(struct kvm_userspace_memory_region *mem) 709static int check_memory_region_flags(struct kvm_userspace_memory_region *mem)
@@ -730,10 +725,24 @@ static struct kvm_memslots *install_new_memslots(struct kvm *kvm,
730{ 725{
731 struct kvm_memslots *old_memslots = kvm->memslots; 726 struct kvm_memslots *old_memslots = kvm->memslots;
732 727
733 update_memslots(slots, new, kvm->memslots->generation); 728 /*
729 * Set the low bit in the generation, which disables SPTE caching
730 * until the end of synchronize_srcu_expedited.
731 */
732 WARN_ON(old_memslots->generation & 1);
733 slots->generation = old_memslots->generation + 1;
734
735 update_memslots(slots, new);
734 rcu_assign_pointer(kvm->memslots, slots); 736 rcu_assign_pointer(kvm->memslots, slots);
735 synchronize_srcu_expedited(&kvm->srcu); 737 synchronize_srcu_expedited(&kvm->srcu);
736 738
739 /*
740 * Increment the new memslot generation a second time. This prevents
741 * vm exits that race with memslot updates from caching a memslot
742 * generation that will (potentially) be valid forever.
743 */
744 slots->generation++;
745
737 kvm_arch_memslots_updated(kvm); 746 kvm_arch_memslots_updated(kvm);
738 747
739 return old_memslots; 748 return old_memslots;