Commit 7460fb4a authored by Avi Kivity's avatar Avi Kivity

KVM: Fix simultaneous NMIs

If simultaneous NMIs happen, we're supposed to queue the second
and next (collapsing them), but currently we sometimes collapse
the second into the first.

Fix by using a counter for pending NMIs instead of a bool; since
the counter limit depends on whether the processor is currently
in an NMI handler, which can only be checked in vcpu context
(via the NMI mask), we add a new KVM_REQ_NMI to request recalculation
of the counter.
Signed-off-by: default avatarAvi Kivity <avi@redhat.com>
Signed-off-by: default avatarMarcelo Tosatti <mtosatti@redhat.com>
parent 1cd196ea
...@@ -413,8 +413,9 @@ struct kvm_vcpu_arch { ...@@ -413,8 +413,9 @@ struct kvm_vcpu_arch {
u32 tsc_catchup_mult; u32 tsc_catchup_mult;
s8 tsc_catchup_shift; s8 tsc_catchup_shift;
bool nmi_pending; atomic_t nmi_queued; /* unprocessed asynchronous NMIs */
bool nmi_injected; unsigned nmi_pending; /* NMI queued after currently running handler */
bool nmi_injected; /* Trying to inject an NMI this entry */
struct mtrr_state_type mtrr_state; struct mtrr_state_type mtrr_state;
u32 pat; u32 pat;
......
...@@ -83,6 +83,7 @@ static u64 __read_mostly efer_reserved_bits = ~((u64)EFER_SCE); ...@@ -83,6 +83,7 @@ static u64 __read_mostly efer_reserved_bits = ~((u64)EFER_SCE);
static void update_cr8_intercept(struct kvm_vcpu *vcpu); static void update_cr8_intercept(struct kvm_vcpu *vcpu);
static int kvm_dev_ioctl_get_supported_cpuid(struct kvm_cpuid2 *cpuid, static int kvm_dev_ioctl_get_supported_cpuid(struct kvm_cpuid2 *cpuid,
struct kvm_cpuid_entry2 __user *entries); struct kvm_cpuid_entry2 __user *entries);
static void process_nmi(struct kvm_vcpu *vcpu);
struct kvm_x86_ops *kvm_x86_ops; struct kvm_x86_ops *kvm_x86_ops;
EXPORT_SYMBOL_GPL(kvm_x86_ops); EXPORT_SYMBOL_GPL(kvm_x86_ops);
...@@ -359,8 +360,8 @@ void kvm_propagate_fault(struct kvm_vcpu *vcpu, struct x86_exception *fault) ...@@ -359,8 +360,8 @@ void kvm_propagate_fault(struct kvm_vcpu *vcpu, struct x86_exception *fault)
void kvm_inject_nmi(struct kvm_vcpu *vcpu) void kvm_inject_nmi(struct kvm_vcpu *vcpu)
{ {
kvm_make_request(KVM_REQ_EVENT, vcpu); atomic_inc(&vcpu->arch.nmi_queued);
vcpu->arch.nmi_pending = 1; kvm_make_request(KVM_REQ_NMI, vcpu);
} }
EXPORT_SYMBOL_GPL(kvm_inject_nmi); EXPORT_SYMBOL_GPL(kvm_inject_nmi);
...@@ -2827,6 +2828,7 @@ static int kvm_vcpu_ioctl_x86_set_mce(struct kvm_vcpu *vcpu, ...@@ -2827,6 +2828,7 @@ static int kvm_vcpu_ioctl_x86_set_mce(struct kvm_vcpu *vcpu,
static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu, static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
struct kvm_vcpu_events *events) struct kvm_vcpu_events *events)
{ {
process_nmi(vcpu);
events->exception.injected = events->exception.injected =
vcpu->arch.exception.pending && vcpu->arch.exception.pending &&
!kvm_exception_is_soft(vcpu->arch.exception.nr); !kvm_exception_is_soft(vcpu->arch.exception.nr);
...@@ -2844,7 +2846,7 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu, ...@@ -2844,7 +2846,7 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
KVM_X86_SHADOW_INT_MOV_SS | KVM_X86_SHADOW_INT_STI); KVM_X86_SHADOW_INT_MOV_SS | KVM_X86_SHADOW_INT_STI);
events->nmi.injected = vcpu->arch.nmi_injected; events->nmi.injected = vcpu->arch.nmi_injected;
events->nmi.pending = vcpu->arch.nmi_pending; events->nmi.pending = vcpu->arch.nmi_pending != 0;
events->nmi.masked = kvm_x86_ops->get_nmi_mask(vcpu); events->nmi.masked = kvm_x86_ops->get_nmi_mask(vcpu);
events->nmi.pad = 0; events->nmi.pad = 0;
...@@ -2864,6 +2866,7 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu, ...@@ -2864,6 +2866,7 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
| KVM_VCPUEVENT_VALID_SHADOW)) | KVM_VCPUEVENT_VALID_SHADOW))
return -EINVAL; return -EINVAL;
process_nmi(vcpu);
vcpu->arch.exception.pending = events->exception.injected; vcpu->arch.exception.pending = events->exception.injected;
vcpu->arch.exception.nr = events->exception.nr; vcpu->arch.exception.nr = events->exception.nr;
vcpu->arch.exception.has_error_code = events->exception.has_error_code; vcpu->arch.exception.has_error_code = events->exception.has_error_code;
...@@ -4763,7 +4766,7 @@ int kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int inc_eip) ...@@ -4763,7 +4766,7 @@ int kvm_inject_realmode_interrupt(struct kvm_vcpu *vcpu, int irq, int inc_eip)
kvm_set_rflags(vcpu, ctxt->eflags); kvm_set_rflags(vcpu, ctxt->eflags);
if (irq == NMI_VECTOR) if (irq == NMI_VECTOR)
vcpu->arch.nmi_pending = false; vcpu->arch.nmi_pending = 0;
else else
vcpu->arch.interrupt.pending = false; vcpu->arch.interrupt.pending = false;
...@@ -5572,7 +5575,7 @@ static void inject_pending_event(struct kvm_vcpu *vcpu) ...@@ -5572,7 +5575,7 @@ static void inject_pending_event(struct kvm_vcpu *vcpu)
/* try to inject new event if pending */ /* try to inject new event if pending */
if (vcpu->arch.nmi_pending) { if (vcpu->arch.nmi_pending) {
if (kvm_x86_ops->nmi_allowed(vcpu)) { if (kvm_x86_ops->nmi_allowed(vcpu)) {
vcpu->arch.nmi_pending = false; --vcpu->arch.nmi_pending;
vcpu->arch.nmi_injected = true; vcpu->arch.nmi_injected = true;
kvm_x86_ops->set_nmi(vcpu); kvm_x86_ops->set_nmi(vcpu);
} }
...@@ -5604,10 +5607,26 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu) ...@@ -5604,10 +5607,26 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
} }
} }
static void process_nmi(struct kvm_vcpu *vcpu)
{
unsigned limit = 2;
/*
* x86 is limited to one NMI running, and one NMI pending after it.
* If an NMI is already in progress, limit further NMIs to just one.
* Otherwise, allow two (and we'll inject the first one immediately).
*/
if (kvm_x86_ops->get_nmi_mask(vcpu) || vcpu->arch.nmi_injected)
limit = 1;
vcpu->arch.nmi_pending += atomic_xchg(&vcpu->arch.nmi_queued, 0);
vcpu->arch.nmi_pending = min(vcpu->arch.nmi_pending, limit);
kvm_make_request(KVM_REQ_EVENT, vcpu);
}
static int vcpu_enter_guest(struct kvm_vcpu *vcpu) static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
{ {
int r; int r;
bool nmi_pending;
bool req_int_win = !irqchip_in_kernel(vcpu->kvm) && bool req_int_win = !irqchip_in_kernel(vcpu->kvm) &&
vcpu->run->request_interrupt_window; vcpu->run->request_interrupt_window;
...@@ -5647,6 +5666,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) ...@@ -5647,6 +5666,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
} }
if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu)) if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu))
record_steal_time(vcpu); record_steal_time(vcpu);
if (kvm_check_request(KVM_REQ_NMI, vcpu))
process_nmi(vcpu);
} }
...@@ -5654,19 +5675,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) ...@@ -5654,19 +5675,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
if (unlikely(r)) if (unlikely(r))
goto out; goto out;
/*
* An NMI can be injected between local nmi_pending read and
* vcpu->arch.nmi_pending read inside inject_pending_event().
* But in that case, KVM_REQ_EVENT will be set, which makes
* the race described above benign.
*/
nmi_pending = ACCESS_ONCE(vcpu->arch.nmi_pending);
if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) { if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
inject_pending_event(vcpu); inject_pending_event(vcpu);
/* enable NMI/IRQ window open exits if needed */ /* enable NMI/IRQ window open exits if needed */
if (nmi_pending) if (vcpu->arch.nmi_pending)
kvm_x86_ops->enable_nmi_window(vcpu); kvm_x86_ops->enable_nmi_window(vcpu);
else if (kvm_cpu_has_interrupt(vcpu) || req_int_win) else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
kvm_x86_ops->enable_irq_window(vcpu); kvm_x86_ops->enable_irq_window(vcpu);
...@@ -6374,7 +6387,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) ...@@ -6374,7 +6387,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu) int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
{ {
vcpu->arch.nmi_pending = false; atomic_set(&vcpu->arch.nmi_queued, 0);
vcpu->arch.nmi_pending = 0;
vcpu->arch.nmi_injected = false; vcpu->arch.nmi_injected = false;
vcpu->arch.switch_db_regs = 0; vcpu->arch.switch_db_regs = 0;
...@@ -6649,7 +6663,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu) ...@@ -6649,7 +6663,7 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
!vcpu->arch.apf.halted) !vcpu->arch.apf.halted)
|| !list_empty_careful(&vcpu->async_pf.done) || !list_empty_careful(&vcpu->async_pf.done)
|| vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED || vcpu->arch.mp_state == KVM_MP_STATE_SIPI_RECEIVED
|| vcpu->arch.nmi_pending || || atomic_read(&vcpu->arch.nmi_queued) ||
(kvm_arch_interrupt_allowed(vcpu) && (kvm_arch_interrupt_allowed(vcpu) &&
kvm_cpu_has_interrupt(vcpu)); kvm_cpu_has_interrupt(vcpu));
} }
......
...@@ -49,6 +49,7 @@ ...@@ -49,6 +49,7 @@
#define KVM_REQ_EVENT 11 #define KVM_REQ_EVENT 11
#define KVM_REQ_APF_HALT 12 #define KVM_REQ_APF_HALT 12
#define KVM_REQ_STEAL_UPDATE 13 #define KVM_REQ_STEAL_UPDATE 13
#define KVM_REQ_NMI 14
#define KVM_USERSPACE_IRQ_SOURCE_ID 0 #define KVM_USERSPACE_IRQ_SOURCE_ID 0
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment