KVM fixes for 3.2.17
Subsystems (6)
cover KVM fixes for 3.2.17
See individual patches for details.
Alex Williamson (1):
KVM: lock slots_lock around device assignment
Avi Kivity (1):
KVM: Ensure all vcpus are consistent with in-kernel irqchip settings
Christian Borntraeger (1):
KVM: s390: Sanitize fpc registers for KVM_SET_FPU
Jens Freimann (1):
KVM: s390: do store status after handling STOP_ON_STOP bit
Marcelo Tosatti (1):
KVM: VMX: vmx_set_cr0 expects kvm->srcu locked
Nadav Har'El (1):
KVM: nVMX: Fix erroneous exception bitmap check
Takuya Yoshikawa (1):
KVM: mmu_notifier: Flush TLBs before releasing mmu_lock
arch/ia64/kvm/kvm-ia64.c | 5 +++++ arch/s390/kvm/intercept.c | 20 ++++++++++++-------- arch/s390/kvm/kvm-s390.c | 2 +- arch/x86/kvm/vmx.c | 4 +++- arch/x86/kvm/x86.c | 8 ++++++++ include/linux/kvm_host.h | 7 +++++++ virt/kvm/iommu.c | 23 +++++++++++++++-------- virt/kvm/kvm_main.c | 23 ++++++++++++++--------- 8 files changed, 65 insertions(+), 27 deletions(-)
Signature · 1 line
1.7.6.4
PATCH 1/7 KVM: mmu_notifier: Flush TLBs before releasing mmu_lock
From: Takuya Yoshikawa <[email protected]>
(cherry picked from commit 565f3be2174611f364405bbea2d86e153c2e7e78
Other threads may process the same page in that small window and skip TLB flush and then return before these functions do flush.
--- virt/kvm/kvm_main.c | 19 ++++++++++--------- 1 files changed, 10 insertions(+), 9 deletions(-)
virt/kvm/kvm_main.c +10 -9
@@ -289,15 +289,15 @@ static void kvm_mmu_notifier_invalidate_page(struct mmu_notifier *mn,*/ idx =srcu_read_lock (&kvm->srcu);spin_lock (&kvm->mmu_lock); + kvm->mmu_notifier_seq++; need_tlb_flush =kvm_unmap_hva (kvm, address) | kvm->tlbs_dirty; -spin_unlock (&kvm->mmu_lock); -srcu_read_unlock (&kvm->srcu, idx); -/* we've to flush the tlb before the pages can be freed */ if (need_tlb_flush)kvm_flush_remote_tlbs (kvm); +spin_unlock (&kvm->mmu_lock); +srcu_read_unlock (&kvm->srcu, idx); } static void kvm_mmu_notifier_change_pte(struct mmu_notifier *mn,
@@ -335,12 +335,12 @@ static void kvm_mmu_notifier_invalidate_range_start(struct mmu_notifier *mn,for (; start < end; start +=PAGE_SIZE ) need_tlb_flush |=kvm_unmap_hva (kvm, start); need_tlb_flush |= kvm->tlbs_dirty; -spin_unlock (&kvm->mmu_lock); -srcu_read_unlock (&kvm->srcu, idx); -/* we've to flush the tlb before the pages can be freed */ if (need_tlb_flush)kvm_flush_remote_tlbs (kvm); + +spin_unlock (&kvm->mmu_lock); +srcu_read_unlock (&kvm->srcu, idx); } static void kvm_mmu_notifier_invalidate_range_end(struct mmu_notifier *mn,
@@ -378,13 +378,14 @@ static int kvm_mmu_notifier_clear_flush_young(struct mmu_notifier *mn, idx =srcu_read_lock (&kvm->srcu);spin_lock (&kvm->mmu_lock); - young =kvm_age_hva (kvm, address); -spin_unlock (&kvm->mmu_lock); -srcu_read_unlock (&kvm->srcu, idx); + young =kvm_age_hva (kvm, address);if (young)kvm_flush_remote_tlbs (kvm); +spin_unlock (&kvm->mmu_lock); +srcu_read_unlock (&kvm->srcu, idx); +return young; }
Signature · 1 line
1.7.6.4
PATCH 2/7 KVM: Ensure all vcpus are consistent with in-kernel irqchip settings
In reply to Marcelo TosattiFrom: Avi Kivity <[email protected]>
(cherry picked from commit 3e515705a1f46beb1c942bb8043c16f8ac7b1e9e)
If some vcpus are created before KVM_CREATE_IRQCHIP, then irqchip_in_kernel() and vcpu->arch.apic will be inconsistent, leading to potential NULL pointer dereferences.
Fix by:
- ensuring that no vcpus are installed when KVM_CREATE_IRQCHIP is called
- ensuring that a vcpu has an apic if it is installed after KVM_CREATE_IRQCHIP
This is somewhat long winded because vcpu->arch.apic is created without kvm->lock held.
Based on earlier patch by Michael Ellerman.
--- arch/ia64/kvm/kvm-ia64.c | 5 +++++ arch/x86/kvm/x86.c | 8 ++++++++ include/linux/kvm_host.h | 7 +++++++ virt/kvm/kvm_main.c | 4 ++++ 4 files changed, 24 insertions(+), 0 deletions(-)
arch/ia64/kvm/kvm-ia64.c +5 -0
@@ -1169,6 +1169,11 @@ out:#define PALE_RESET_ENTRY 0x80000000ffffffb0UL +bool kvm_vcpu_compatible (struct kvm_vcpu *vcpu) +{ +return irqchip_in_kernel (vcpu->kcm) == (vcpu->arch.apic !=NULL ); +} +int kvm_arch_vcpu_init (struct kvm_vcpu *vcpu) {struct kvm_vcpu *v;
arch/x86/kvm/x86.c +8 -0
@@ -3579,6 +3579,9 @@ long kvm_arch_vm_ioctl(struct file *filp, r = -EEXIST ;if (kvm->arch.vpic) goto create_irqchip_unlock; + r = -EINVAL ; +if (atomic_read (&kvm->online_vcpus)) + goto create_irqchip_unlock; r = -ENOMEM ; vpic =kvm_create_pic (kvm);if (vpic) {
@@ -6486,6 +6489,11 @@ void kvm_arch_check_processor_compat(void *rtn) kvm_x86_ops->check_processor_compatibility (rtn); } +bool kvm_vcpu_compatible (struct kvm_vcpu *vcpu) +{ +return irqchip_in_kernel (vcpu->kvm) == (vcpu->arch.apic !=NULL ); +} +int kvm_arch_vcpu_init (struct kvm_vcpu *vcpu) {struct page *page;
include/linux/kvm_host.h +7 -0
@@ -744,6 +744,13 @@ static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) {return vcpu->kvm->bsp_vcpu_id == vcpu->vcpu_id; } + +bool kvm_vcpu_compatible (struct kvm_vcpu *vcpu); + +#else + +static inline bool kvm_vcpu_compatible (struct kvm_vcpu *vcpu) {return true; } +#endif #ifdef __KVM_HAVE_DEVICE_ASSIGNMENT
virt/kvm/kvm_main.c +4 -0
@@ -1667,6 +1667,10 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) goto vcpu_destroy;mutex_lock (&kvm->lock); +if (!kvm_vcpu_compatible (vcpu)) { + r = -EINVAL ; + goto unlock_vcpu_destroy; + }if (atomic_read (&kvm->online_vcpus) ==KVM_MAX_VCPUS ) { r = -EINVAL ; goto unlock_vcpu_destroy;
Signature · 1 line
1.7.6.4
PATCH 3/7 KVM: lock slots_lock around device assignment
In reply to Marcelo TosattiFrom: Alex Williamson <[email protected]>
(cherry picked from commit 21a1416a1c945c5aeaeaf791b63c64926018eb77)
As pointed out by Jason Baron, when assigning a device to a guest we first set the iommu domain pointer, which enables mapping and unmapping of memory slots to the iommu. This leaves a window where this path is enabled, but we haven't synchronized the iommu mappings to the existing memory slots. Thus a slot being removed at that point could send us down unexpected code paths removing non-existent pinnings and iommu mappings. Take the slots_lock around creating the iommu domain and initial mappings as well as around iommu teardown to avoid this race.
--- virt/kvm/iommu.c | 23 +++++++++++++++-------- 1 files changed, 15 insertions(+), 8 deletions(-)
virt/kvm/iommu.c +15 -8
@@ -239,9 +239,13 @@ int kvm_iommu_map_guest(struct kvm *kvm)return -ENODEV ; } +mutex_lock (&kvm->slots_lock); + kvm->arch.iommu_domain =iommu_domain_alloc (&pci_bus_type); -if (!kvm->arch.iommu_domain) -return -ENOMEM ; +if (!kvm->arch.iommu_domain) { + r = -ENOMEM ; + goto out_unlock; + }if (!allow_unsafe_assigned_interrupts && !iommu_domain_has_cap(kvm->arch.iommu_domain,
@@ -252,17 +256,16 @@ int kvm_iommu_map_guest(struct kvm *kvm) " module option.\n", __func__);iommu_domain_free (kvm->arch.iommu_domain); kvm->arch.iommu_domain =NULL ; -return -EPERM ; + r = -EPERM ; + goto out_unlock; } r =kvm_iommu_map_memslots (kvm);if (r) - goto out_unmap; - -return 0 ; +kvm_iommu_unmap_memslots (kvm); -out_unmap: -kvm_iommu_unmap_memslots (kvm); +out_unlock: +mutex_unlock (&kvm->slots_lock);return r; }
@@ -338,7 +341,11 @@ int kvm_iommu_unmap_guest(struct kvm *kvm)if (!domain)return 0 ; +mutex_lock (&kvm->slots_lock);kvm_iommu_unmap_memslots (kvm); + kvm->arch.iommu_domain =NULL ; +mutex_unlock (&kvm->slots_lock); +iommu_domain_free (domain);return 0 ; }
Signature · 1 line
1.7.6.4
PATCH 4/7 KVM: nVMX: Fix erroneous exception bitmap check
In reply to Marcelo TosattiFrom: Nadav Har'El <[email protected]>
(cherry picked from commit 9587190107d0c0cbaccbf7bf6b0245d29095a9ae)
The code which checks whether to inject a pagefault to L1 or L2 (in nested VMX) was wrong, incorrect in how it checked the PF_VECTOR bit. Thanks to Dan Carpenter for spotting this.
--- arch/x86/kvm/vmx.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/kvm/vmx.c +1 -1
@@ -1677,7 +1677,7 @@ static int nested_pf_handled(struct kvm_vcpu *vcpu)struct vmcs12 *vmcs12 =get_vmcs12 (vcpu);/* TODO: also check PFEC_MATCH/MASK, not just EB.PF. */ -if (!(vmcs12->exception_bitmap &PF_VECTOR )) +if (!(vmcs12->exception_bitmap & (1u <<PF_VECTOR )))return 0 ;nested_vmx_vmexit (vcpu);
Signature · 1 line
1.7.6.4
PATCH 5/7 KVM: VMX: vmx_set_cr0 expects kvm->srcu locked
In reply to Marcelo Tosatti(cherry picked from commit 7a4f5ad051e02139a9f1c0f7f4b1acb88915852b)
vmx_set_cr0 is called from vcpu run context, therefore it expects kvm->srcu to be held (for setting up the real-mode TSS).
--- arch/x86/kvm/vmx.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
arch/x86/kvm/vmx.c +2 -0
@@ -3915,7 +3915,9 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)vmcs_write16 (VIRTUAL_PROCESSOR_ID , vmx->vpid); vmx->vcpu.arch.cr0 =X86_CR0_NW |X86_CR0_CD |X86_CR0_ET ; + vcpu->srcu_idx =srcu_read_lock (&vcpu->kvm->srcu);vmx_set_cr0 (&vmx->vcpu,kvm_read_cr0 (vcpu));/* enter rmode */ +srcu_read_unlock (&vcpu->kvm->srcu, vcpu->srcu_idx);vmx_set_cr4 (&vmx->vcpu,0 );vmx_set_efer (&vmx->vcpu,0 );vmx_fpu_activate (&vmx->vcpu);
Signature · 1 line
1.7.6.4
PATCH 6/7 KVM: s390: do store status after handling STOP_ON_STOP bit
In reply to Marcelo TosattiFrom: Jens Freimann <[email protected]>
(cherry picked from commit 9e0d5473e2f0ba2d2fe9dab9408edef3060b710e)
In handle_stop() handle the stop bit before doing the store status as described for "Stop and Store Status" in the Principles of Operation. We have to give up the local_int.lock before calling kvm store status since it calls gmap_fault() which might sleep. Since local_int.lock only protects local_int.* and not guest memory we can give up the lock.
--- arch/s390/kvm/intercept.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-)
arch/s390/kvm/intercept.c +12 -8
@@ -133,13 +133,6 @@ static int handle_stop(struct kvm_vcpu *vcpu) vcpu->stat.exit_stop_request++;spin_lock_bh (&vcpu->arch.local_int.lock); -if (vcpu->arch.local_int.action_bits &ACTION_STORE_ON_STOP ) { - vcpu->arch.local_int.action_bits &= ~ACTION_STORE_ON_STOP ; - rc =kvm_s390_vcpu_store_status (vcpu, -KVM_S390_STORE_STATUS_NOADDR ); -if (rc >=0 ) - rc = -EOPNOTSUPP ; - }if (vcpu->arch.local_int.action_bits &ACTION_RELOADVCPU_ON_STOP ) { vcpu->arch.local_int.action_bits &= ~ACTION_RELOADVCPU_ON_STOP ;
@@ -155,7 +148,18 @@ static int handle_stop(struct kvm_vcpu *vcpu) rc = -EOPNOTSUPP ; } -spin_unlock_bh (&vcpu->arch.local_int.lock); +if (vcpu->arch.local_int.action_bits &ACTION_STORE_ON_STOP ) { + vcpu->arch.local_int.action_bits &= ~ACTION_STORE_ON_STOP ; +/* store status must be called unlocked. Since local_int.lock +* only protects local_int.* and not guest memory we can give +* up the lock here */ +spin_unlock_bh (&vcpu->arch.local_int.lock); + rc =kvm_s390_vcpu_store_status (vcpu, +KVM_S390_STORE_STATUS_NOADDR ); +if (rc >=0 ) + rc = -EOPNOTSUPP ; + }else +spin_unlock_bh (&vcpu->arch.local_int.lock);return rc; }
Signature · 1 line
1.7.6.4
PATCH 7/7 KVM: s390: Sanitize fpc registers for KVM_SET_FPU
In reply to Marcelo TosattiFrom: Christian Borntraeger <[email protected]>
(cherry picked from commit 851755871c1f3184f4124c466e85881f17fa3226)
commit 7eef87dc99e419b1cc051e4417c37e4744d7b661 (KVM: s390: fix register setting) added a load of the floating point control register to the KVM_SET_FPU path. Lets make sure that the fpc is valid.
--- arch/s390/kvm/kvm-s390.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/s390/kvm/kvm-s390.c +1 -1
@@ -418,7 +418,7 @@ int kvm_arch_vcpu_ioctl_get_sregs(struct kvm_vcpu *vcpu,int kvm_arch_vcpu_ioctl_set_fpu (struct kvm_vcpu *vcpu,struct kvm_fpu *fpu) {memcpy (&vcpu->arch.guest_fpregs.fprs, &fpu->fprs,sizeof (fpu->fprs)); - vcpu->arch.guest_fpregs.fpc = fpu->fpc; + vcpu->arch.guest_fpregs.fpc = fpu->fpc &FPC_VALID_MASK ;restore_fp_regs (&vcpu->arch.guest_fpregs);return 0 ; }
Signature · 1 line
1.7.6.4
Quoted 1 line · Marcelo Tosatti
See individual patches for details.
[...]
These came a little too late for 3.2.18, but I've queued them up now.
Ben.
Signature · 2 lines
Ben Hutchings You can't have everything. Where would you put it?