diff mbox

[1/3] KVM: nVMX: Fix virtual interrupt delivery injection

Message ID 1405573011-41330-1-git-send-email-wanpeng.li@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wanpeng Li July 17, 2014, 4:56 a.m. UTC
This patch fix bug reported in https://bugzilla.kernel.org/show_bug.cgi?id=73331, 
after the patch http://www.spinics.net/lists/kvm/msg105230.html applied, there is
some progress and the L2 can boot up, however, slowly. The original idea of this 
fix vid injection patch is from "Zhang, Yang Z" <yang.z.zhang@intel.com>.

Interrupt which delivered by vid should be injected to L1 by L0 if current is in 
L1, or should be injected to L2 by L0 through the old injection way if L1 doesn't 
have set VM_EXIT_ACK_INTR_ON_EXIT. The current logic doen't consider these cases. 
This patch fix it by vid intr to L1 if current is L1 or L2 through old injection 
way if L1 doen't have VM_EXIT_ACK_INTR_ON_EXIT set.

Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
Signed-off-by: "Zhang, Yang Z" <yang.z.zhang@intel.com>
---
 arch/x86/kvm/vmx.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Paolo Bonzini July 17, 2014, 8:55 a.m. UTC | #1
Il 17/07/2014 06:56, Wanpeng Li ha scritto:
> This patch fix bug reported in https://bugzilla.kernel.org/show_bug.cgi?id=73331,
> after the patch http://www.spinics.net/lists/kvm/msg105230.html applied, there is
> some progress and the L2 can boot up, however, slowly. The original idea of this
> fix vid injection patch is from "Zhang, Yang Z" <yang.z.zhang@intel.com>.
>
> Interrupt which delivered by vid should be injected to L1 by L0 if current is in
> L1, or should be injected to L2 by L0 through the old injection way if L1 doesn't
> have set VM_EXIT_ACK_INTR_ON_EXIT. The current logic doen't consider these cases.
> This patch fix it by vid intr to L1 if current is L1 or L2 through old injection
> way if L1 doen't have VM_EXIT_ACK_INTR_ON_EXIT set.
>
> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
> Signed-off-by: "Zhang, Yang Z" <yang.z.zhang@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 021d84a..ad36646 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -7112,8 +7112,22 @@ static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
>  {
>  	if (max_irr == -1)
>  		return;
> -
> -	vmx_set_rvi(max_irr);
> +	if (!is_guest_mode(vcpu)) {
> +		vmx_set_rvi(max_irr);
> +	} else if (is_guest_mode(vcpu) && !nested_exit_on_intr(vcpu)) {
> +		/*
> +		 * Fall back to old way to inject the interrupt since there
> +		 * is no vAPIC-v for L2.
> +		 */
> +		if (vcpu->arch.exception.pending ||
> +				vcpu->arch.nmi_injected ||
> +				vcpu->arch.interrupt.pending)
> +			return;
> +		else if (vmx_interrupt_allowed(vcpu)) {
> +			kvm_queue_interrupt(vcpu, max_irr, false);
> +			vmx_inject_irq(vcpu);
> +		}
> +	}
>  }
>
>  static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
>

What hypervisor did you test with? nested_exit_on_intr(vcpu) will return 
true for both Xen and KVM (nested_exit_on_intr is not the same thing as 
ACK_INTR_ON_EXIT).

Paolo
--
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
Zhang, Yang Z July 17, 2014, 9:03 a.m. UTC | #2
Paolo Bonzini wrote on 2014-07-17:
> Il 17/07/2014 06:56, Wanpeng Li ha scritto:
>> This patch fix bug reported in
>> https://bugzilla.kernel.org/show_bug.cgi?id=73331, after the patch
>> http://www.spinics.net/lists/kvm/msg105230.html applied, there is some
>> progress and the L2 can boot up, however, slowly. The original idea of
>> this fix vid injection patch is from "Zhang, Yang Z"
>> <yang.z.zhang@intel.com>.
>> 
>> Interrupt which delivered by vid should be injected to L1 by L0 if
>> current is in L1, or should be injected to L2 by L0 through the old
>> injection way if L1 doesn't have set VM_EXIT_ACK_INTR_ON_EXIT. The
>> current logic doen't consider these cases. This patch fix it by vid
>> intr to L1 if current is L1 or L2 through old injection way if L1
>> doen't have VM_EXIT_ACK_INTR_ON_EXIT set.
>> 
>> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>> Signed-off-by: "Zhang, Yang Z" <yang.z.zhang@intel.com>
>> ---
>>  arch/x86/kvm/vmx.c | 18 ++++++++++++++++--
>>  1 file changed, 16 insertions(+), 2 deletions(-)
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index
>> 021d84a..ad36646 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -7112,8 +7112,22 @@ static void vmx_hwapic_irr_update(struct
>> kvm_vcpu *vcpu, int max_irr)  {
>>  	if (max_irr == -1)
>>  		return;
>> -
>> -	vmx_set_rvi(max_irr);
>> +	if (!is_guest_mode(vcpu)) {
>> +		vmx_set_rvi(max_irr);
>> +	} else if (is_guest_mode(vcpu) && !nested_exit_on_intr(vcpu)) {
>> +		/*
>> +		 * Fall back to old way to inject the interrupt since there
>> +		 * is no vAPIC-v for L2.
>> +		 */
>> +		if (vcpu->arch.exception.pending ||
>> +				vcpu->arch.nmi_injected ||
>> +				vcpu->arch.interrupt.pending)
>> +			return;
>> +		else if (vmx_interrupt_allowed(vcpu)) {
>> +			kvm_queue_interrupt(vcpu, max_irr, false);
>> +			vmx_inject_irq(vcpu);
>> +		}
>> +	}
>>  }
>>  
>>  static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64
>> *eoi_exit_bitmap)
>> 
> 
> What hypervisor did you test with? nested_exit_on_intr(vcpu) will

Jailhouse will clear External-interrupt exiting bit. Am I right? Jan.

> return true for both Xen and KVM (nested_exit_on_intr is not the same
> thing as ACK_INTR_ON_EXIT).

I guess he want to say External-interrupt exiting bit not ACK_INTR_ON_EXIT. 

> 
> Paolo


Best regards,
Yang


--
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
Wanpeng Li July 17, 2014, 9:11 a.m. UTC | #3
On Thu, Jul 17, 2014 at 09:03:01AM +0000, Zhang, Yang Z wrote:
>Paolo Bonzini wrote on 2014-07-17:
>> Il 17/07/2014 06:56, Wanpeng Li ha scritto:
>>> This patch fix bug reported in
>>> https://bugzilla.kernel.org/show_bug.cgi?id=73331, after the patch
>>> http://www.spinics.net/lists/kvm/msg105230.html applied, there is some
>>> progress and the L2 can boot up, however, slowly. The original idea of
>>> this fix vid injection patch is from "Zhang, Yang Z"
>>> <yang.z.zhang@intel.com>.
>>> 
>>> Interrupt which delivered by vid should be injected to L1 by L0 if
>>> current is in L1, or should be injected to L2 by L0 through the old
>>> injection way if L1 doesn't have set VM_EXIT_ACK_INTR_ON_EXIT. The
>>> current logic doen't consider these cases. This patch fix it by vid
>>> intr to L1 if current is L1 or L2 through old injection way if L1
>>> doen't have VM_EXIT_ACK_INTR_ON_EXIT set.
>>> 
>>> Signed-off-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>>> Signed-off-by: "Zhang, Yang Z" <yang.z.zhang@intel.com>
>>> ---
>>>  arch/x86/kvm/vmx.c | 18 ++++++++++++++++--
>>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index
>>> 021d84a..ad36646 100644
>>> --- a/arch/x86/kvm/vmx.c
>>> +++ b/arch/x86/kvm/vmx.c
>>> @@ -7112,8 +7112,22 @@ static void vmx_hwapic_irr_update(struct
>>> kvm_vcpu *vcpu, int max_irr)  {
>>>  	if (max_irr == -1)
>>>  		return;
>>> -
>>> -	vmx_set_rvi(max_irr);
>>> +	if (!is_guest_mode(vcpu)) {
>>> +		vmx_set_rvi(max_irr);
>>> +	} else if (is_guest_mode(vcpu) && !nested_exit_on_intr(vcpu)) {
>>> +		/*
>>> +		 * Fall back to old way to inject the interrupt since there
>>> +		 * is no vAPIC-v for L2.
>>> +		 */
>>> +		if (vcpu->arch.exception.pending ||
>>> +				vcpu->arch.nmi_injected ||
>>> +				vcpu->arch.interrupt.pending)
>>> +			return;
>>> +		else if (vmx_interrupt_allowed(vcpu)) {
>>> +			kvm_queue_interrupt(vcpu, max_irr, false);
>>> +			vmx_inject_irq(vcpu);
>>> +		}
>>> +	}
>>>  }
>>>  
>>>  static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64
>>> *eoi_exit_bitmap)
>>> 
>> 
>> What hypervisor did you test with? nested_exit_on_intr(vcpu) will
>
>Jailhouse will clear External-interrupt exiting bit. Am I right? Jan.
>
>> return true for both Xen and KVM (nested_exit_on_intr is not the same
>> thing as ACK_INTR_ON_EXIT).
>
>I guess he want to say External-interrupt exiting bit not ACK_INTR_ON_EXIT. 
>

Ah yes, a typo here. 

Regards,
Wanpeng Li 

>> 
>> Paolo
>
>
>Best regards,
>Yang
>
--
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
Paolo Bonzini July 17, 2014, 10:43 a.m. UTC | #4
Il 17/07/2014 11:11, Wanpeng Li ha scritto:
>>> >> What hypervisor did you test with? nested_exit_on_intr(vcpu) will
>> >
>> >Jailhouse will clear External-interrupt exiting bit. Am I right? Jan.
>> >
>>> >> return true for both Xen and KVM (nested_exit_on_intr is not the same
>>> >> thing as ACK_INTR_ON_EXIT).
>> >
>> >I guess he want to say External-interrupt exiting bit not ACK_INTR_ON_EXIT.
>> >
> Ah yes, a typo here.

Ok, please repost this patch together with your version of patch 2.

Leave aside patch 3 for now, as I think the original use-after-free 
patch was wrong.

Paolo
--
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
Wanpeng Li July 17, 2014, 11:08 a.m. UTC | #5
On Thu, Jul 17, 2014 at 12:43:58PM +0200, Paolo Bonzini wrote:
>Il 17/07/2014 11:11, Wanpeng Li ha scritto:
>>>>>> What hypervisor did you test with? nested_exit_on_intr(vcpu) will
>>>>
>>>>Jailhouse will clear External-interrupt exiting bit. Am I right? Jan.
>>>>
>>>>>> return true for both Xen and KVM (nested_exit_on_intr is not the same
>>>>>> thing as ACK_INTR_ON_EXIT).
>>>>
>>>>I guess he want to say External-interrupt exiting bit not ACK_INTR_ON_EXIT.
>>>>
>>Ah yes, a typo here.
>
>Ok, please repost this patch together with your version of patch 2.

Just send out the version two of 1/3 and 2/3. 

>
>Leave aside patch 3 for now, as I think the original use-after-free
>patch was wrong.

Any proposal is appreciated. ;-)

Regards,
Wanpeng Li 

>
>Paolo
--
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
diff mbox

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 021d84a..ad36646 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7112,8 +7112,22 @@  static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr)
 {
 	if (max_irr == -1)
 		return;
-
-	vmx_set_rvi(max_irr);
+	if (!is_guest_mode(vcpu)) {
+		vmx_set_rvi(max_irr);
+	} else if (is_guest_mode(vcpu) && !nested_exit_on_intr(vcpu)) {
+		/*
+		 * Fall back to old way to inject the interrupt since there
+		 * is no vAPIC-v for L2.
+		 */
+		if (vcpu->arch.exception.pending ||
+				vcpu->arch.nmi_injected ||
+				vcpu->arch.interrupt.pending)
+			return;
+		else if (vmx_interrupt_allowed(vcpu)) {
+			kvm_queue_interrupt(vcpu, max_irr, false);
+			vmx_inject_irq(vcpu);
+		}
+	}
 }
 
 static void vmx_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)