Skip to contentSkip to conversation

KVM: nVMX: Skip PF interception check when queuing during nested run

PATCH In review Jan Kiszka 6 participants 11 messages Apr 28, 2013 – May 3, 2013

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 KVM: nVMX: Skip PF interception check when queuing during nested run

From: Jan Kiszka <[email protected]>

While a nested run is pending, vmx_queue_exception is only called to requeue exceptions that were previously picked up via vmx_cancel_injection. Therefore, we must not check for PF interception by L1, possibly causing a bogus nested vmexit.

Signed-off-by Jan Kiszka <[email protected]>

---

This and the KVM_REQ_IMMEDIATE_EXIT fix allows me to boot an L2 Linux without problems. Yongjie, please check if it resolves your issue(s) as well.

 arch/x86/kvm/vmx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/kvm/vmx.c +1 -1
index d663a59..45eb949 100644
@@ -1917,7 +1917,7 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
 	u32 intr_info = nr | INTR_INFO_VALID_MASK;
 
 	if (nr == PF_VECTOR && is_guest_mode(vcpu) &&
-		nested_pf_handled(vcpu))
+	    !vmx->nested.nested_run_pending && nested_pf_handled(vcpu))
 		return;
 
 	if (has_error_code) {
Signature · 1 line
1.7.3.4
#1 lore raw
Quoted 8 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

While a nested run is pending, vmx_queue_exception is only called to requeue exceptions that were previously picked up via vmx_cancel_injection. Therefore, we must not check for PF interception by L1, possibly causing a bogus nested vmexit.

Signed-off-by Jan Kiszka <[email protected]>

Applied thanks. We should get rid of nested_run_pending state, but re-executing instruction if emulation cannot be complete.

Quoted 24 lines

---

This and the KVM_REQ_IMMEDIATE_EXIT fix allows me to boot an L2 Linux without problems. Yongjie, please check if it resolves your issue(s) as well.

 arch/x86/kvm/vmx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/x86/kvm/vmx.c +1 -1
index d663a59..45eb949 100644
@@ -1917,7 +1917,7 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
 	u32 intr_info = nr | INTR_INFO_VALID_MASK;
 
 	if (nr == PF_VECTOR && is_guest_mode(vcpu) &&
-		nested_pf_handled(vcpu))
+	    !vmx->nested.nested_run_pending && nested_pf_handled(vcpu))
 		return;
 
 	if (has_error_code) {
Signature · 1 line
1.7.3.4

--
Gleb.

#2 lore raw
In reply to Jan Kiszka
Quoted 16 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Jan Kiszka
Sent: Sunday, April 28, 2013 3:25 PM
To: Gleb Natapov; Marcelo Tosatti
Cc: kvm; Nakajima, Jun; Ren, Yongjie
Subject: [PATCH] KVM: nVMX: Skip PF interception check when queuing during nested run

From: Jan Kiszka <[email protected]>

While a nested run is pending, vmx_queue_exception is only called to requeue exceptions that were previously picked up via vmx_cancel_injection. Therefore, we must not check for PF interception by L1, possibly causing a bogus nested vmexit.

Quoted 7 lines
Signed-off-by Jan Kiszka <[email protected]>

---

This and the KVM_REQ_IMMEDIATE_EXIT fix allows me to boot an L2 Linux without problems. Yongjie, please check if it resolves your issue(s) as well.

The two patches can fix my issue. When both of them are applied, I can have more tests against next branch.

#3 lore raw
Quoted 19 lines · Yongjie
Quoted 19 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Jan Kiszka
Sent: Sunday, April 28, 2013 3:25 PM
To: Gleb Natapov; Marcelo Tosatti
Cc: kvm; Nakajima, Jun; Ren, Yongjie
Subject: [PATCH] KVM: nVMX: Skip PF interception check when queuing during nested run

From: Jan Kiszka <[email protected]>

While a nested run is pending, vmx_queue_exception is only called to requeue exceptions that were previously picked up via vmx_cancel_injection. Therefore, we must not check for PF interception by L1, possibly causing a bogus nested vmexit.

Signed-off-by Jan Kiszka <[email protected]>

---

Quoted 6 lines
Quoted 4 lines

This and the KVM_REQ_IMMEDIATE_EXIT fix allows me to boot an L2 Linux without problems. Yongjie, please check if it resolves your issue(s) as well.

The two patches can fix my issue. When both of them are applied, I can have more tests against next branch.

They are both applied now.

--
Gleb.

#4 lore raw
Quoted 31 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Gleb Natapov
Sent: Sunday, April 28, 2013 10:34 PM
To: Ren, Yongjie
Cc: Jan Kiszka; Marcelo Tosatti; kvm; Nakajima, Jun
Subject: Re: [PATCH] KVM: nVMX: Skip PF interception check when queuing during nested run

Quoted 2 lines · Yongjie
Quoted 2 lines

-----Original Message-----
From: [email protected]

[mailto:[email protected]]

Quoted 14 lines
Quoted 14 lines

On Behalf Of Jan Kiszka
Sent: Sunday, April 28, 2013 3:25 PM
To: Gleb Natapov; Marcelo Tosatti
Cc: kvm; Nakajima, Jun; Ren, Yongjie
Subject: [PATCH] KVM: nVMX: Skip PF interception check when queuing during nested run

From: Jan Kiszka <[email protected]>

While a nested run is pending, vmx_queue_exception is only called to requeue exceptions that were previously picked up via vmx_cancel_injection. Therefore, we must not check for PF interception by L1, possibly causing a bogus nested vmexit.

Quoted 4 lines
Quoted 4 lines
Signed-off-by Jan Kiszka <[email protected]>

---

This and the KVM_REQ_IMMEDIATE_EXIT fix allows me to boot an L2

Quoted 9 lines

Linux

Quoted 4 lines
Quoted 3 lines

without problems. Yongjie, please check if it resolves your issue(s) as well.

The two patches can fix my issue. When both of them are applied, I can

have

Quoted 1 line

more tests against next branch.

They are both applied now.

There's some bug in Jan's patch "Rework request for immediate exit". When I said 2 patches can fix my issue, I meant his original two patches. "Check KVM_REQ_IMMEDIATE_EXIT after enable_irq_window" works for me. "Rework request for immediate exit" patch is buggy.
In L1, I can get the following error. (also some NMI in L2.) (BTW, I'll have holidays this week. I may not track this issue this week.)

[  167.248015] sending NMI to all CPUs:
[  167.252260] NMI backtrace for cpu 1
[  167.253007] CPU 1
[  167.253007] Pid: 0, comm: swapper/1 Tainted: GF            3.8.5 #1 Bochs Bochs
[  167.253007] RIP: 0010:[<ffffffff81045606>]  [<ffffffff81045606>] native_safe_halt+0x6/0x10
[  167.253007] RSP: 0018:ffff880290d51ed8  EFLAGS: 00000246
[  167.253007] RAX: 0000000000000000 RBX: ffff880290d50010 RCX: 0140000000000000
[  167.253007] RDX: 0000000000000000 RSI: 0140000000000000 RDI: 0000000000000086
[  167.253007] RBP: ffff880290d51ed8 R08: 0000000000000000 R09: 0000000000000000
[  167.253007] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
[  167.253007] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[  167.253007] FS:  0000000000000000(0000) GS:ffff88029fc40000(0000) knlGS:0000000000000000
[  167.253007] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  167.253007] CR2: ffffffffff600400 CR3: 000000028f12d000 CR4: 00000000000427e0
[  167.253007] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  167.253007] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  167.253007] Process swapper/1 (pid: 0, threadinfo ffff880290d50000, task ffff880290d49740)
[  167.253007] Stack:
[  167.253007]  ffff880290d51ef8 ffffffff8101d5cf ffff880290d50010 ffffffff81ce0680
[  167.253007]  ffff880290d51f28 ffffffff8101ce99 ffff880290d51f18 1de4884102b62f69
[  167.253007]  0000000000000000 0000000000000000 ffff880290d51f48 ffffffff81643595
[  167.253007] Call Trace:
[  167.253007]  [<ffffffff8101d5cf>] default_idle+0x4f/0x1a0
[  167.253007]  [<ffffffff8101ce99>] cpu_idle+0xd9/0x120
[  167.253007]  [<ffffffff81643595>] start_secondary+0x24c/0x24e
[  167.253007] Code: 00 00 00 00 00 55 48 89 e5 fa c9 c3 66 0f 1f 84 00 00 00 00 00 55 48 89 e5 fb c9 c3 66 0f 1f 84 00 00 00 00 00 55 48 89 e5 fb f4 <c9> c3 0f 1f 84 00 00 00 00 00 55 48 89 e5 f4 c9 c3 66 0f 1f 84
[  167.248015] NMI backtrace for cpu 3
[  167.248015] CPU 3
[  167.248015] Pid: 0, comm: swapper/3 Tainted: GF            3.8.5 #1 Bochs Bochs
[  167.248015] RIP: 0010:[<ffffffff810454ca>]  [<ffffffff810454ca>] native_write_msr_safe+0xa/0x10

.......

author
#5 lore raw
Quoted 75 lines · Yongjie
Quoted 31 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Gleb Natapov
Sent: Sunday, April 28, 2013 10:34 PM
To: Ren, Yongjie
Cc: Jan Kiszka; Marcelo Tosatti; kvm; Nakajima, Jun
Subject: Re: [PATCH] KVM: nVMX: Skip PF interception check when queuing during nested run

Quoted 2 lines · Yongjie
Quoted 2 lines

-----Original Message-----
From: [email protected]

[mailto:[email protected]]

Quoted 14 lines
Quoted 14 lines

On Behalf Of Jan Kiszka
Sent: Sunday, April 28, 2013 3:25 PM
To: Gleb Natapov; Marcelo Tosatti
Cc: kvm; Nakajima, Jun; Ren, Yongjie
Subject: [PATCH] KVM: nVMX: Skip PF interception check when queuing during nested run

From: Jan Kiszka <[email protected]>

While a nested run is pending, vmx_queue_exception is only called to requeue exceptions that were previously picked up via vmx_cancel_injection. Therefore, we must not check for PF interception by L1, possibly causing a bogus nested vmexit.

Quoted 4 lines
Quoted 4 lines
Signed-off-by Jan Kiszka <[email protected]>

---

This and the KVM_REQ_IMMEDIATE_EXIT fix allows me to boot an L2

Quoted 9 lines

Linux

Quoted 4 lines
Quoted 3 lines

without problems. Yongjie, please check if it resolves your issue(s) as well.

The two patches can fix my issue. When both of them are applied, I can

have

Quoted 1 line

more tests against next branch.

They are both applied now.

There's some bug in Jan's patch "Rework request for immediate exit". When I said 2 patches can fix my issue, I meant his original two patches. "Check KVM_REQ_IMMEDIATE_EXIT after enable_irq_window" works for me. "Rework request for immediate exit" patch is buggy.
In L1, I can get the following error. (also some NMI in L2.) (BTW, I'll have holidays this week. I may not track this issue this week.)

[  167.252260] NMI backtrace for cpu 1
[  167.253007] CPU 1
[  167.253007] Pid: 0, comm: swapper/1 Tainted: GF            3.8.5 #1 Bochs Bochs
[  167.253007] RIP: 0010:[<ffffffff81045606>]  [<ffffffff81045606>] native_safe_halt+0x6/0x10
[  167.253007] RSP: 0018:ffff880290d51ed8  EFLAGS: 00000246
[  167.253007] RAX: 0000000000000000 RBX: ffff880290d50010 RCX: 0140000000000000
[  167.253007] RDX: 0000000000000000 RSI: 0140000000000000 RDI: 0000000000000086
[  167.253007] RBP: ffff880290d51ed8 R08: 0000000000000000 R09: 0000000000000000
[  167.253007] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
[  167.253007] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[  167.253007] FS:  0000000000000000(0000) GS:ffff88029fc40000(0000) knlGS:0000000000000000
[  167.253007] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  167.253007] CR2: ffffffffff600400 CR3: 000000028f12d000 CR4: 00000000000427e0
[  167.253007] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  167.253007] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  167.253007] Process swapper/1 (pid: 0, threadinfo ffff880290d50000, task ffff880290d49740)
[  167.253007] Stack:
[  167.253007]  ffff880290d51ef8 ffffffff8101d5cf ffff880290d50010 ffffffff81ce0680
[  167.253007]  ffff880290d51f28 ffffffff8101ce99 ffff880290d51f18 1de4884102b62f69
[  167.253007]  0000000000000000 0000000000000000 ffff880290d51f48 ffffffff81643595
[  167.253007] Call Trace:
[  167.253007]  [<ffffffff8101d5cf>] default_idle+0x4f/0x1a0
[  167.253007]  [<ffffffff8101ce99>] cpu_idle+0xd9/0x120
[  167.253007]  [<ffffffff81643595>] start_secondary+0x24c/0x24e
[  167.253007] Code: 00 00 00 00 00 55 48 89 e5 fa c9 c3 66 0f 1f 84 00 00 00 00 00 55 48 89 e5 fb c9 c3 66 0f 1f 84 00 00 00 00 00 55 48 89 e5 fb f4 <c9> c3 0f 1f 84 00 00 00 00 00 55 48 89 e5 f4 c9 c3 66 0f 1f 84
[  167.248015] NMI backtrace for cpu 3
[  167.248015] CPU 3
[  167.248015] Pid: 0, comm: swapper/3 Tainted: GF            3.8.5 #1 Bochs Bochs
[  167.248015] RIP: 0010:[<ffffffff810454ca>]  [<ffffffff810454ca>] native_write_msr_safe+0xa/0x10
Quoted 2 lines

.......

Argh, of course: We use enable_irq_window also for the NMI window in certain scenarios. So enable_nmi_window must be changed accordingly. Will send a patch.

Thanks,
Jan

author
#6 lore raw

PATCH 1/1 KVM: x86: Account for failing enable_irq_window for NMI window request

From: Jan Kiszka <[email protected]>

With VMX, enable_irq_window can now return -EBUSY, in which case an immediate exit shall be requested before entering the guest. Account for this also in enable_nmi_window which uses enable_irq_window in absence of vnmi support, e.g.

Signed-off-by Jan Kiszka <[email protected]>
---
 arch/x86/include/asm/kvm_host.h |    2 +-
 arch/x86/kvm/svm.c              |    5 +++--
 arch/x86/kvm/vmx.c              |   16 +++++++---------
 arch/x86/kvm/x86.c              |    3 ++-
 4 files changed, 13 insertions(+), 13 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -1
index ec14b72..3741c65 100644
@@ -695,7 +695,7 @@ struct kvm_x86_ops {
 	int (*nmi_allowed)(struct kvm_vcpu *vcpu);
 	bool (*get_nmi_mask)(struct kvm_vcpu *vcpu);
 	void (*set_nmi_mask)(struct kvm_vcpu *vcpu, bool masked);
-	void (*enable_nmi_window)(struct kvm_vcpu *vcpu);
+	int (*enable_nmi_window)(struct kvm_vcpu *vcpu);
 	int (*enable_irq_window)(struct kvm_vcpu *vcpu);
 	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
 	int (*vm_has_apicv)(struct kvm *kvm);
arch/x86/kvm/svm.c +3 -2
index 7f896cb..3421d5a 100644
@@ -3649,13 +3649,13 @@ static int enable_irq_window(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static void enable_nmi_window(struct kvm_vcpu *vcpu)
+static int enable_nmi_window(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	if ((svm->vcpu.arch.hflags & (HF_NMI_MASK | HF_IRET_MASK))
 	    == HF_NMI_MASK)
-		return; /* IRET will cause a vm exit */
+		return 0; /* IRET will cause a vm exit */
 
 	/*
 	 * Something prevents NMI from been injected. Single step over possible

@@ -3664,6 +3664,7 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
 	svm->nmi_singlestep = true;
 	svm->vmcb->save.rflags |= (X86_EFLAGS_TF | X86_EFLAGS_RF);
 	update_db_bp_intercept(vcpu);
+	return 0;
 }
 
 static int svm_set_tss_addr(struct kvm *kvm, unsigned int addr)
arch/x86/kvm/vmx.c +7 -9
index 55a1aa0..2f7af9c 100644
@@ -4417,22 +4417,20 @@ static int enable_irq_window(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static void enable_nmi_window(struct kvm_vcpu *vcpu)
+static int enable_nmi_window(struct kvm_vcpu *vcpu)
 {
 	u32 cpu_based_vm_exec_control;
 
-	if (!cpu_has_virtual_nmis()) {
-		enable_irq_window(vcpu);
-		return;
-	}
+	if (!cpu_has_virtual_nmis())
+		return enable_irq_window(vcpu);
+
+	if (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_STI)
+		return enable_irq_window(vcpu);
 
-	if (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_STI) {
-		enable_irq_window(vcpu);
-		return;
-	}
 	cpu_based_vm_exec_control = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
 	cpu_based_vm_exec_control |= CPU_BASED_VIRTUAL_NMI_PENDING;
 	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control);
+	return 0;
 }
 
 static void vmx_inject_irq(struct kvm_vcpu *vcpu)
arch/x86/kvm/x86.c +2 -1
index 8747fef..6974ca8 100644
@@ -5754,7 +5754,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 
 		/* enable NMI/IRQ window open exits if needed */
 		if (vcpu->arch.nmi_pending)
-			kvm_x86_ops->enable_nmi_window(vcpu);
+			req_immediate_exit =
+				kvm_x86_ops->enable_nmi_window(vcpu);
 		else if (kvm_cpu_has_injectable_intr(vcpu) || req_int_win)
 			req_immediate_exit =
 				kvm_x86_ops->enable_irq_window(vcpu) != 0;
Signature · 1 line
1.7.3.4
maintainer
#7 lore raw

Re: [PATCH] KVM: x86: Account for failing enable_irq_window for NMI window request

Il 28/04/2013 18:40, Jan Kiszka ha scritto:

arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_vcpu *vcpu) · line 5758

Quoted 11 lines
arch/x86/kvm/x86.c +2 -1
index 8747fef..6974ca8 100644
@@ -5754,7 +5754,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 
 		/* enable NMI/IRQ window open exits if needed */
 		if (vcpu->arch.nmi_pending)
-			kvm_x86_ops->enable_nmi_window(vcpu);
+			req_immediate_exit =
+				kvm_x86_ops->enable_nmi_window(vcpu);

!= 0 for consistency with below?

Paolo

Quoted 4 lines
arch/x86/kvm/x86.c +0 -1
 		else if (kvm_cpu_has_injectable_intr(vcpu) || req_int_win)
 			req_immediate_exit =
 				kvm_x86_ops->enable_irq_window(vcpu) != 0;
-- 1.7.3.4
#8 lore raw

PATCH v2 1/1 KVM: x86: Account for failing enable_irq_window for NMI window request

With VMX, enable_irq_window can now return -EBUSY, in which case an immediate exit shall be requested before entering the guest. Account for this also in enable_nmi_window which uses enable_irq_window in absence of vnmi support, e.g.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- check return code of enable_nmi_window against 0 instead of using it directly

 arch/x86/include/asm/kvm_host.h |    2 +-
 arch/x86/kvm/svm.c              |    5 +++--
 arch/x86/kvm/vmx.c              |   16 +++++++---------
 arch/x86/kvm/x86.c              |    3 ++-
 4 files changed, 13 insertions(+), 13 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -1
index ec14b72..3741c65 100644
@@ -695,7 +695,7 @@ struct kvm_x86_ops {
 	int (*nmi_allowed)(struct kvm_vcpu *vcpu);
 	bool (*get_nmi_mask)(struct kvm_vcpu *vcpu);
 	void (*set_nmi_mask)(struct kvm_vcpu *vcpu, bool masked);
-	void (*enable_nmi_window)(struct kvm_vcpu *vcpu);
+	int (*enable_nmi_window)(struct kvm_vcpu *vcpu);
 	int (*enable_irq_window)(struct kvm_vcpu *vcpu);
 	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
 	int (*vm_has_apicv)(struct kvm *kvm);
arch/x86/kvm/svm.c +3 -2
index 7f896cb..3421d5a 100644
@@ -3649,13 +3649,13 @@ static int enable_irq_window(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static void enable_nmi_window(struct kvm_vcpu *vcpu)
+static int enable_nmi_window(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	if ((svm->vcpu.arch.hflags & (HF_NMI_MASK | HF_IRET_MASK))
 	    == HF_NMI_MASK)
-		return; /* IRET will cause a vm exit */
+		return 0; /* IRET will cause a vm exit */
 
 	/*
 	 * Something prevents NMI from been injected. Single step over possible

@@ -3664,6 +3664,7 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
 	svm->nmi_singlestep = true;
 	svm->vmcb->save.rflags |= (X86_EFLAGS_TF | X86_EFLAGS_RF);
 	update_db_bp_intercept(vcpu);
+	return 0;
 }
 
 static int svm_set_tss_addr(struct kvm *kvm, unsigned int addr)
arch/x86/kvm/vmx.c +7 -9
index 55a1aa0..2f7af9c 100644
@@ -4417,22 +4417,20 @@ static int enable_irq_window(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static void enable_nmi_window(struct kvm_vcpu *vcpu)
+static int enable_nmi_window(struct kvm_vcpu *vcpu)
 {
 	u32 cpu_based_vm_exec_control;
 
-	if (!cpu_has_virtual_nmis()) {
-		enable_irq_window(vcpu);
-		return;
-	}
+	if (!cpu_has_virtual_nmis())
+		return enable_irq_window(vcpu);
+
+	if (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_STI)
+		return enable_irq_window(vcpu);
 
-	if (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_STI) {
-		enable_irq_window(vcpu);
-		return;
-	}
 	cpu_based_vm_exec_control = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
 	cpu_based_vm_exec_control |= CPU_BASED_VIRTUAL_NMI_PENDING;
 	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control);
+	return 0;
 }
 
 static void vmx_inject_irq(struct kvm_vcpu *vcpu)
arch/x86/kvm/x86.c +2 -1
index 8747fef..24724b42 100644
@@ -5754,7 +5754,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 
 		/* enable NMI/IRQ window open exits if needed */
 		if (vcpu->arch.nmi_pending)
-			kvm_x86_ops->enable_nmi_window(vcpu);
+			req_immediate_exit =
+				kvm_x86_ops->enable_nmi_window(vcpu) != 0;
 		else if (kvm_cpu_has_injectable_intr(vcpu) || req_int_win)
 			req_immediate_exit =
 				kvm_x86_ops->enable_irq_window(vcpu) != 0;
maintainer Reviewed-by on v2 1/1
#9 lore raw

Re: [PATCH v2] KVM: x86: Account for failing enable_irq_window for NMI window request

Il 29/04/2013 16:46, Jan Kiszka ha scritto:

Quoted 6 lines

With VMX, enable_irq_window can now return -EBUSY, in which case an immediate exit shall be requested before entering the guest. Account for this also in enable_nmi_window which uses enable_irq_window in absence of vnmi support, e.g.

Signed-off-by Jan Kiszka <[email protected]>
Reviewed-by Paolo Bonzini <[email protected]>
Quoted 102 lines

---

Changes in v2:
- check return code of enable_nmi_window against 0 instead of using it directly

 arch/x86/include/asm/kvm_host.h |    2 +-
 arch/x86/kvm/svm.c              |    5 +++--
 arch/x86/kvm/vmx.c              |   16 +++++++---------
 arch/x86/kvm/x86.c              |    3 ++-
 4 files changed, 13 insertions(+), 13 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -1
index ec14b72..3741c65 100644
@@ -695,7 +695,7 @@ struct kvm_x86_ops {
 	int (*nmi_allowed)(struct kvm_vcpu *vcpu);
 	bool (*get_nmi_mask)(struct kvm_vcpu *vcpu);
 	void (*set_nmi_mask)(struct kvm_vcpu *vcpu, bool masked);
-	void (*enable_nmi_window)(struct kvm_vcpu *vcpu);
+	int (*enable_nmi_window)(struct kvm_vcpu *vcpu);
 	int (*enable_irq_window)(struct kvm_vcpu *vcpu);
 	void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
 	int (*vm_has_apicv)(struct kvm *kvm);
arch/x86/kvm/svm.c +3 -2
index 7f896cb..3421d5a 100644
@@ -3649,13 +3649,13 @@ static int enable_irq_window(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static void enable_nmi_window(struct kvm_vcpu *vcpu)
+static int enable_nmi_window(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	if ((svm->vcpu.arch.hflags & (HF_NMI_MASK | HF_IRET_MASK))
 	    == HF_NMI_MASK)
-		return; /* IRET will cause a vm exit */
+		return 0; /* IRET will cause a vm exit */
 
 	/*
 	 * Something prevents NMI from been injected. Single step over possible

@@ -3664,6 +3664,7 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
 	svm->nmi_singlestep = true;
 	svm->vmcb->save.rflags |= (X86_EFLAGS_TF | X86_EFLAGS_RF);
 	update_db_bp_intercept(vcpu);
+	return 0;
 }
 
 static int svm_set_tss_addr(struct kvm *kvm, unsigned int addr)
arch/x86/kvm/vmx.c +7 -9
index 55a1aa0..2f7af9c 100644
@@ -4417,22 +4417,20 @@ static int enable_irq_window(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static void enable_nmi_window(struct kvm_vcpu *vcpu)
+static int enable_nmi_window(struct kvm_vcpu *vcpu)
 {
 	u32 cpu_based_vm_exec_control;
 
-	if (!cpu_has_virtual_nmis()) {
-		enable_irq_window(vcpu);
-		return;
-	}
+	if (!cpu_has_virtual_nmis())
+		return enable_irq_window(vcpu);
+
+	if (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_STI)
+		return enable_irq_window(vcpu);
 
-	if (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_STI) {
-		enable_irq_window(vcpu);
-		return;
-	}
 	cpu_based_vm_exec_control = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
 	cpu_based_vm_exec_control |= CPU_BASED_VIRTUAL_NMI_PENDING;
 	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control);
+	return 0;
 }
 
 static void vmx_inject_irq(struct kvm_vcpu *vcpu)
arch/x86/kvm/x86.c +2 -1
index 8747fef..24724b42 100644
@@ -5754,7 +5754,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 
 		/* enable NMI/IRQ window open exits if needed */
 		if (vcpu->arch.nmi_pending)
-			kvm_x86_ops->enable_nmi_window(vcpu);
+			req_immediate_exit =
+				kvm_x86_ops->enable_nmi_window(vcpu) != 0;
 		else if (kvm_cpu_has_injectable_intr(vcpu) || req_int_win)
 			req_immediate_exit =
 				kvm_x86_ops->enable_irq_window(vcpu) != 0;
Applied
#10 lore raw

Re: [PATCH v2] KVM: x86: Account for failing enable_irq_window for NMI window request

Quoted 9 lines · Paolo Bonzini

Il 29/04/2013 16:46, Jan Kiszka ha scritto:

Quoted 6 lines

With VMX, enable_irq_window can now return -EBUSY, in which case an immediate exit shall be requested before entering the guest. Account for this also in enable_nmi_window which uses enable_irq_window in absence of vnmi support, e.g.

Signed-off-by Jan Kiszka <[email protected]>
Reviewed-by Paolo Bonzini <[email protected]>

Applied, thanks.