All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01
@ 2014-08-01  8:12 Wanpeng Li
  2014-08-01  8:12 ` [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when APICv is in use Wanpeng Li
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Wanpeng Li @ 2014-08-01  8:12 UTC (permalink / raw)
  To: Paolo Bonzini, Jan Kiszka
  Cc: Marcelo Tosatti, Gleb Natapov, Bandan Das, Zhang Yang, kvm,
	linux-kernel, Wanpeng Li

External interrupt will cause L1 vmexit w/ reason external interrupt when L2 is 
running. Then L1 will pick up the interrupt through vmcs12 if L1 set the ack 
interrupt bit. Commit 77b0f5d (KVM: nVMX: Ack and write vector info to intr_info
if L1 asks us to) get intr that belongs to L1 before load vmcs01 which is wrong, 
especially this lead to the obvious L1 ack APICv behavior weired since APICv 
is for L1 instead of L2. This patch fix it by ack intr after load vmcs01.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
 arch/x86/kvm/vmx.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e618f34..b8122b3 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8754,14 +8754,6 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
 	prepare_vmcs12(vcpu, vmcs12, exit_reason, exit_intr_info,
 		       exit_qualification);
 
-	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
-	    && nested_exit_intr_ack_set(vcpu)) {
-		int irq = kvm_cpu_get_interrupt(vcpu);
-		WARN_ON(irq < 0);
-		vmcs12->vm_exit_intr_info = irq |
-			INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR;
-	}
-
 	trace_kvm_nested_vmexit_inject(vmcs12->vm_exit_reason,
 				       vmcs12->exit_qualification,
 				       vmcs12->idt_vectoring_info_field,
@@ -8771,6 +8763,14 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
 
 	vmx_load_vmcs01(vcpu);
 
+	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
+	    && nested_exit_intr_ack_set(vcpu)) {
+		int irq = kvm_cpu_get_interrupt(vcpu);
+		WARN_ON(irq < 0);
+		vmcs12->vm_exit_intr_info = irq |
+			INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR;
+	}
+
 	vm_entry_controls_init(vmx, vmcs_read32(VM_ENTRY_CONTROLS));
 	vm_exit_controls_init(vmx, vmcs_read32(VM_EXIT_CONTROLS));
 	vmx_segment_cache_clear(vmx);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when APICv is in use
  2014-08-01  8:12 [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01 Wanpeng Li
@ 2014-08-01  8:12 ` Wanpeng Li
  2014-08-04  2:03   ` Liu, RongrongX
  2014-08-04 12:51   ` Paolo Bonzini
  2014-08-04  2:01 ` [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01 Liu, RongrongX
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 8+ messages in thread
From: Wanpeng Li @ 2014-08-01  8:12 UTC (permalink / raw)
  To: Paolo Bonzini, Jan Kiszka
  Cc: Marcelo Tosatti, Gleb Natapov, Bandan Das, Zhang Yang, kvm,
	linux-kernel, Wanpeng Li

After commit 77b0f5d (KVM: nVMX: Ack and write vector info to intr_info
if L1 asks us to), "Acknowledge interrupt on exit" behavior can be
emulated. To do so, KVM will ask the APIC for the interrupt vector if
during a nested vmexit if VM_EXIT_ACK_INTR_ON_EXIT is set.  With APICv,
kvm_get_apic_interrupt would return -1 and give the following WARNING:

Call Trace:
 [<ffffffff81493563>] dump_stack+0x49/0x5e
 [<ffffffff8103f0eb>] warn_slowpath_common+0x7c/0x96
 [<ffffffffa059709a>] ? nested_vmx_vmexit+0xa4/0x233 [kvm_intel]
 [<ffffffff8103f11a>] warn_slowpath_null+0x15/0x17
 [<ffffffffa059709a>] nested_vmx_vmexit+0xa4/0x233 [kvm_intel]
 [<ffffffffa0594295>] ? nested_vmx_exit_handled+0x6a/0x39e [kvm_intel]
 [<ffffffffa0537931>] ? kvm_apic_has_interrupt+0x80/0xd5 [kvm]
 [<ffffffffa05972ec>] vmx_check_nested_events+0xc3/0xd3 [kvm_intel]
 [<ffffffffa051ebe9>] inject_pending_event+0xd0/0x16e [kvm]
 [<ffffffffa051efa0>] vcpu_enter_guest+0x319/0x704 [kvm]

If enabling APIC-v, all interrupts to L1 are delivered through APIC-v.
But when L2 is running, external interrupt will casue L1 vmexit with
reason external interrupt. Then L1 will pick up the interrupt through
vmcs12. when L1 ack the interrupt, since the APIC-v is enabled when
L1 is running, so APIC-v hardware still will do vEOI updating. The problem
is that the interrupt is delivered not through APIC-v hardware, this means
SVI/RVI/vPPR are not setting, but hardware required them when doing vEOI
updating. The solution is that, when L1 tried to pick up the interrupt
from vmcs12, then hypervisor will help to update the SVI/RVI/vPPR to make
sure the following vEOI updating and vPPR updating corrently.
    
Also, since interrupt is delivered through vmcs12, so APIC-v hardware will
not cleare vIRR and hypervisor need to clear it before L1 running.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Suggested-by: "Zhang, Yang Z" <yang.z.zhang@intel.com>
Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
---
 arch/x86/kvm/lapic.c | 18 ++++++++++++++++++
 arch/x86/kvm/lapic.h |  1 +
 arch/x86/kvm/vmx.c   | 10 ++++++++++
 3 files changed, 29 insertions(+)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 3855103..06942b9 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -534,6 +534,24 @@ static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
 	apic_update_ppr(apic);
 }
 
+int kvm_lapic_ack_apicv(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->arch.apic;
+	int vec;
+
+	vec = kvm_apic_has_interrupt(vcpu);
+
+	if (vec == -1)
+		return vec;
+
+	apic_set_vector(vec, apic->regs + APIC_ISR);
+	apic_update_ppr(apic);
+	apic_clear_vector(vec, apic->regs + APIC_IRR);
+
+	return vec;
+}
+EXPORT_SYMBOL_GPL(kvm_lapic_ack_apicv);
+
 int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)
 {
 	return dest == 0xff || kvm_apic_id(apic) == dest;
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index 6a11845..ead1392 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -169,5 +169,6 @@ static inline bool kvm_apic_has_events(struct kvm_vcpu *vcpu)
 }
 
 bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector);
+int kvm_lapic_ack_apicv(struct kvm_vcpu *vcpu);
 
 #endif
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index b8122b3..c604f3c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8766,6 +8766,16 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
 	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
 	    && nested_exit_intr_ack_set(vcpu)) {
 		int irq = kvm_cpu_get_interrupt(vcpu);
+
+		if (irq < 0 && kvm_apic_vid_enabled(vcpu->kvm)) {
+			irq = kvm_lapic_ack_apicv(vcpu);
+			if (irq >= 0) {
+				vmx_hwapic_isr_update(vcpu->kvm, irq);
+				/* try to update RVI */
+				kvm_make_request(KVM_REQ_EVENT, vcpu);
+			}
+		}
+
 		WARN_ON(irq < 0);
 		vmcs12->vm_exit_intr_info = irq |
 			INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01
  2014-08-01  8:12 [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01 Wanpeng Li
  2014-08-01  8:12 ` [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when APICv is in use Wanpeng Li
@ 2014-08-04  2:01 ` Liu, RongrongX
  2014-08-04 12:51 ` Paolo Bonzini
  2014-08-04 19:56 ` Davidlohr Bueso
  3 siblings, 0 replies; 8+ messages in thread
From: Liu, RongrongX @ 2014-08-04  2:01 UTC (permalink / raw)
  To: Wanpeng Li, Paolo Bonzini, Jan Kiszka
  Cc: Marcelo Tosatti, Gleb Natapov, Bandan Das, Zhang, Yang Z, kvm,
	linux-kernel



> -----Original Message-----
> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org]
> On Behalf Of Wanpeng Li
> Sent: Friday, August 01, 2014 4:12 PM
> To: Paolo Bonzini; Jan Kiszka
> Cc: Marcelo Tosatti; Gleb Natapov; Bandan Das; Zhang, Yang Z;
> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; Wanpeng Li
> Subject: [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load
> vmcs01
> 
> External interrupt will cause L1 vmexit w/ reason external interrupt when L2
> is running. Then L1 will pick up the interrupt through vmcs12 if L1 set the ack
> interrupt bit. Commit 77b0f5d (KVM: nVMX: Ack and write vector info to
> intr_info if L1 asks us to) get intr that belongs to L1 before load vmcs01 which
> is wrong, especially this lead to the obvious L1 ack APICv behavior weired
> since APICv is for L1 instead of L2. This patch fix it by ack intr after load
> vmcs01.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>

Tested-by:  Liu, RongrongX <rongrongx.liu@intel.com>

> ---
>  arch/x86/kvm/vmx.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index
> e618f34..b8122b3 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8754,14 +8754,6 @@ static void nested_vmx_vmexit(struct kvm_vcpu
> *vcpu, u32 exit_reason,
>  	prepare_vmcs12(vcpu, vmcs12, exit_reason, exit_intr_info,
>  		       exit_qualification);
> 
> -	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
> -	    && nested_exit_intr_ack_set(vcpu)) {
> -		int irq = kvm_cpu_get_interrupt(vcpu);
> -		WARN_ON(irq < 0);
> -		vmcs12->vm_exit_intr_info = irq |
> -			INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR;
> -	}
> -
>  	trace_kvm_nested_vmexit_inject(vmcs12->vm_exit_reason,
>  				       vmcs12->exit_qualification,
>  				       vmcs12->idt_vectoring_info_field, @@ -
> 8771,6 +8763,14 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu,
> u32 exit_reason,
> 
>  	vmx_load_vmcs01(vcpu);
> 
> +	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
> +	    && nested_exit_intr_ack_set(vcpu)) {
> +		int irq = kvm_cpu_get_interrupt(vcpu);
> +		WARN_ON(irq < 0);
> +		vmcs12->vm_exit_intr_info = irq |
> +			INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR;
> +	}
> +
>  	vm_entry_controls_init(vmx, vmcs_read32(VM_ENTRY_CONTROLS));
>  	vm_exit_controls_init(vmx, vmcs_read32(VM_EXIT_CONTROLS));
>  	vmx_segment_cache_clear(vmx);
> --
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in the body of
> a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when APICv is in use
  2014-08-01  8:12 ` [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when APICv is in use Wanpeng Li
@ 2014-08-04  2:03   ` Liu, RongrongX
  2014-08-04 12:51   ` Paolo Bonzini
  1 sibling, 0 replies; 8+ messages in thread
From: Liu, RongrongX @ 2014-08-04  2:03 UTC (permalink / raw)
  To: Wanpeng Li, Paolo Bonzini, Jan Kiszka
  Cc: Marcelo Tosatti, Gleb Natapov, Bandan Das, Zhang, Yang Z, kvm,
	linux-kernel



> -----Original Message-----
> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org]
> On Behalf Of Wanpeng Li
> Sent: Friday, August 01, 2014 4:12 PM
> To: Paolo Bonzini; Jan Kiszka
> Cc: Marcelo Tosatti; Gleb Natapov; Bandan Das; Zhang, Yang Z;
> kvm@vger.kernel.org; linux-kernel@vger.kernel.org; Wanpeng Li
> Subject: [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when
> APICv is in use
> 
> After commit 77b0f5d (KVM: nVMX: Ack and write vector info to intr_info if
> L1 asks us to), "Acknowledge interrupt on exit" behavior can be emulated. To
> do so, KVM will ask the APIC for the interrupt vector if during a nested
> vmexit if VM_EXIT_ACK_INTR_ON_EXIT is set.  With APICv,
> kvm_get_apic_interrupt would return -1 and give the following WARNING:
> 
> Call Trace:
>  [<ffffffff81493563>] dump_stack+0x49/0x5e  [<ffffffff8103f0eb>]
> warn_slowpath_common+0x7c/0x96  [<ffffffffa059709a>] ?
> nested_vmx_vmexit+0xa4/0x233 [kvm_intel]  [<ffffffff8103f11a>]
> warn_slowpath_null+0x15/0x17  [<ffffffffa059709a>]
> nested_vmx_vmexit+0xa4/0x233 [kvm_intel]  [<ffffffffa0594295>] ?
> nested_vmx_exit_handled+0x6a/0x39e [kvm_intel]  [<ffffffffa0537931>] ?
> kvm_apic_has_interrupt+0x80/0xd5 [kvm]  [<ffffffffa05972ec>]
> vmx_check_nested_events+0xc3/0xd3 [kvm_intel]  [<ffffffffa051ebe9>]
> inject_pending_event+0xd0/0x16e [kvm]  [<ffffffffa051efa0>]
> vcpu_enter_guest+0x319/0x704 [kvm]
> 
> If enabling APIC-v, all interrupts to L1 are delivered through APIC-v.
> But when L2 is running, external interrupt will casue L1 vmexit with reason
> external interrupt. Then L1 will pick up the interrupt through vmcs12. when
> L1 ack the interrupt, since the APIC-v is enabled when
> L1 is running, so APIC-v hardware still will do vEOI updating. The problem is
> that the interrupt is delivered not through APIC-v hardware, this means
> SVI/RVI/vPPR are not setting, but hardware required them when doing vEOI
> updating. The solution is that, when L1 tried to pick up the interrupt from
> vmcs12, then hypervisor will help to update the SVI/RVI/vPPR to make sure
> the following vEOI updating and vPPR updating corrently.
> 
> Also, since interrupt is delivered through vmcs12, so APIC-v hardware will not
> cleare vIRR and hypervisor need to clear it before L1 running.
> 
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Suggested-by: "Zhang, Yang Z" <yang.z.zhang@intel.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>

Tested-by:  Liu, RongrongX <rongrongx.liu@intel.com>

> ---
>  arch/x86/kvm/lapic.c | 18 ++++++++++++++++++  arch/x86/kvm/lapic.h |  1
> +
>  arch/x86/kvm/vmx.c   | 10 ++++++++++
>  3 files changed, 29 insertions(+)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index
> 3855103..06942b9 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -534,6 +534,24 @@ static void apic_set_tpr(struct kvm_lapic *apic, u32
> tpr)
>  	apic_update_ppr(apic);
>  }
> 
> +int kvm_lapic_ack_apicv(struct kvm_vcpu *vcpu) {
> +	struct kvm_lapic *apic = vcpu->arch.apic;
> +	int vec;
> +
> +	vec = kvm_apic_has_interrupt(vcpu);
> +
> +	if (vec == -1)
> +		return vec;
> +
> +	apic_set_vector(vec, apic->regs + APIC_ISR);
> +	apic_update_ppr(apic);
> +	apic_clear_vector(vec, apic->regs + APIC_IRR);
> +
> +	return vec;
> +}
> +EXPORT_SYMBOL_GPL(kvm_lapic_ack_apicv);
> +
>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest)  {
>  	return dest == 0xff || kvm_apic_id(apic) == dest; diff --git
> a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h index 6a11845..ead1392
> 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -169,5 +169,6 @@ static inline bool kvm_apic_has_events(struct
> kvm_vcpu *vcpu)  }
> 
>  bool kvm_apic_pending_eoi(struct kvm_vcpu *vcpu, int vector);
> +int kvm_lapic_ack_apicv(struct kvm_vcpu *vcpu);
> 
>  #endif
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index
> b8122b3..c604f3c 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8766,6 +8766,16 @@ static void nested_vmx_vmexit(struct kvm_vcpu
> *vcpu, u32 exit_reason,
>  	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
>  	    && nested_exit_intr_ack_set(vcpu)) {
>  		int irq = kvm_cpu_get_interrupt(vcpu);
> +
> +		if (irq < 0 && kvm_apic_vid_enabled(vcpu->kvm)) {
> +			irq = kvm_lapic_ack_apicv(vcpu);
> +			if (irq >= 0) {
> +				vmx_hwapic_isr_update(vcpu->kvm, irq);
> +				/* try to update RVI */
> +				kvm_make_request(KVM_REQ_EVENT,
> vcpu);
> +			}
> +		}
> +
>  		WARN_ON(irq < 0);
>  		vmcs12->vm_exit_intr_info = irq |
>  			INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR;
> --
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in the body of
> a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when APICv is in use
  2014-08-01  8:12 ` [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when APICv is in use Wanpeng Li
  2014-08-04  2:03   ` Liu, RongrongX
@ 2014-08-04 12:51   ` Paolo Bonzini
  1 sibling, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2014-08-04 12:51 UTC (permalink / raw)
  To: Wanpeng Li, Jan Kiszka
  Cc: Marcelo Tosatti, Gleb Natapov, Bandan Das, Zhang Yang, kvm, linux-kernel

Il 01/08/2014 10:12, Wanpeng Li ha scritto:
> +int kvm_lapic_ack_apicv(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_lapic *apic = vcpu->arch.apic;
> +	int vec;
> +
> +	vec = kvm_apic_has_interrupt(vcpu);
> +
> +	if (vec == -1)
> +		return vec;
> +
> +	apic_set_vector(vec, apic->regs + APIC_ISR);
> +	apic_update_ppr(apic);
> +	apic_clear_vector(vec, apic->regs + APIC_IRR);
> +
> +	return vec;
> +}
> +EXPORT_SYMBOL_GPL(kvm_lapic_ack_apicv);

I would prefer reusing kvm_get_apic_interrupt here (by modifying
kvm_cpu_get_interrupt, apic_set_isr and apic_clear_irr) as I had
sketched in the previous version of the patches.

There is no reason for kvm_cpu_get_interrupt to return -1 if virtual
interrupt delivery is enabled.

Paolo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01
  2014-08-01  8:12 [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01 Wanpeng Li
  2014-08-01  8:12 ` [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when APICv is in use Wanpeng Li
  2014-08-04  2:01 ` [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01 Liu, RongrongX
@ 2014-08-04 12:51 ` Paolo Bonzini
  2014-08-04 19:56 ` Davidlohr Bueso
  3 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2014-08-04 12:51 UTC (permalink / raw)
  To: Wanpeng Li, Jan Kiszka
  Cc: Marcelo Tosatti, Gleb Natapov, Bandan Das, Zhang Yang, kvm, linux-kernel

Il 01/08/2014 10:12, Wanpeng Li ha scritto:
> External interrupt will cause L1 vmexit w/ reason external interrupt when L2 is 
> running. Then L1 will pick up the interrupt through vmcs12 if L1 set the ack 
> interrupt bit. Commit 77b0f5d (KVM: nVMX: Ack and write vector info to intr_info
> if L1 asks us to) get intr that belongs to L1 before load vmcs01 which is wrong, 
> especially this lead to the obvious L1 ack APICv behavior weired since APICv 
> is for L1 instead of L2. This patch fix it by ack intr after load vmcs01.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> ---
>  arch/x86/kvm/vmx.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index e618f34..b8122b3 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8754,14 +8754,6 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
>  	prepare_vmcs12(vcpu, vmcs12, exit_reason, exit_intr_info,
>  		       exit_qualification);
>  
> -	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
> -	    && nested_exit_intr_ack_set(vcpu)) {
> -		int irq = kvm_cpu_get_interrupt(vcpu);
> -		WARN_ON(irq < 0);
> -		vmcs12->vm_exit_intr_info = irq |
> -			INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR;
> -	}
> -
>  	trace_kvm_nested_vmexit_inject(vmcs12->vm_exit_reason,
>  				       vmcs12->exit_qualification,
>  				       vmcs12->idt_vectoring_info_field,
> @@ -8771,6 +8763,14 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason,
>  
>  	vmx_load_vmcs01(vcpu);
>  
> +	if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT)
> +	    && nested_exit_intr_ack_set(vcpu)) {
> +		int irq = kvm_cpu_get_interrupt(vcpu);
> +		WARN_ON(irq < 0);
> +		vmcs12->vm_exit_intr_info = irq |
> +			INTR_INFO_VALID_MASK | INTR_TYPE_EXT_INTR;
> +	}
> +
>  	vm_entry_controls_init(vmx, vmcs_read32(VM_ENTRY_CONTROLS));
>  	vm_exit_controls_init(vmx, vmcs_read32(VM_EXIT_CONTROLS));
>  	vmx_segment_cache_clear(vmx);
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01
  2014-08-01  8:12 [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01 Wanpeng Li
                   ` (2 preceding siblings ...)
  2014-08-04 12:51 ` Paolo Bonzini
@ 2014-08-04 19:56 ` Davidlohr Bueso
  2014-08-05  4:48   ` Wanpeng Li
  3 siblings, 1 reply; 8+ messages in thread
From: Davidlohr Bueso @ 2014-08-04 19:56 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: Paolo Bonzini, Jan Kiszka, Marcelo Tosatti, Gleb Natapov,
	Bandan Das, Zhang Yang, kvm, linux-kernel, freyes

On Fri, 2014-08-01 at 16:12 +0800, Wanpeng Li wrote:
> External interrupt will cause L1 vmexit w/ reason external interrupt when L2 is 
> running. Then L1 will pick up the interrupt through vmcs12 if L1 set the ack 
> interrupt bit. Commit 77b0f5d (KVM: nVMX: Ack and write vector info to intr_info
> if L1 asks us to)

So being that 77b0f5d is in 3.15, we need it for -stable.

>  get intr that belongs to L1 before load vmcs01 which is wrong, 
> especially this lead to the obvious L1 ack APICv behavior weired since APICv 
> is for L1 instead of L2. This patch fix it by ack intr after load vmcs01.
> 
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>

It strikes me that this fixes Felipe's issue:
http://paste.opensuse.org/view/raw/78752586





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01
  2014-08-04 19:56 ` Davidlohr Bueso
@ 2014-08-05  4:48   ` Wanpeng Li
  0 siblings, 0 replies; 8+ messages in thread
From: Wanpeng Li @ 2014-08-05  4:48 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: Paolo Bonzini, Jan Kiszka, Marcelo Tosatti, Gleb Natapov,
	Bandan Das, Zhang Yang, kvm, linux-kernel, freyes

Hi Davidlohr,
On Mon, Aug 04, 2014 at 12:56:36PM -0700, Davidlohr Bueso wrote:
>On Fri, 2014-08-01 at 16:12 +0800, Wanpeng Li wrote:
>> External interrupt will cause L1 vmexit w/ reason external interrupt when L2 is 
>> running. Then L1 will pick up the interrupt through vmcs12 if L1 set the ack 
>> interrupt bit. Commit 77b0f5d (KVM: nVMX: Ack and write vector info to intr_info
>> if L1 asks us to)
>
>So being that 77b0f5d is in 3.15, we need it for -stable.
>
>>  get intr that belongs to L1 before load vmcs01 which is wrong, 
>> especially this lead to the obvious L1 ack APICv behavior weired since APICv 
>> is for L1 instead of L2. This patch fix it by ack intr after load vmcs01.
>> 
>> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>
>It strikes me that this fixes Felipe's issue:
>http://paste.opensuse.org/view/raw/78752586
>

Could you verify if the latest version also fix your issue? Your Tested-by
to the latest version is appreciated.

Regards,
Wanpeng Li 

>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-08-05  4:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-01  8:12 [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01 Wanpeng Li
2014-08-01  8:12 ` [PATCH 2/2] KVM: nVMX: fix acknowledge interrupt on exit when APICv is in use Wanpeng Li
2014-08-04  2:03   ` Liu, RongrongX
2014-08-04 12:51   ` Paolo Bonzini
2014-08-04  2:01 ` [PATCH 1/2] KVM: nVMX: Fix nested vmexit ack intr before load vmcs01 Liu, RongrongX
2014-08-04 12:51 ` Paolo Bonzini
2014-08-04 19:56 ` Davidlohr Bueso
2014-08-05  4:48   ` Wanpeng Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.