linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] KVM: x86: accept userspace interrupt only if no event is injected
@ 2021-07-27 17:06 Paolo Bonzini
  2021-07-27 20:58 ` Sean Christopherson
  0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2021-07-27 17:06 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: stable, Stas Sergeev

Once an exception has been injected, any side effects related to
the exception (such as setting CR2 or DR6) have been taked place.
Therefore, once KVM sets the VM-entry interruption information
field or the AMD EVENTINJ field, the next VM-entry must deliver that
exception.

Pending interrupts are processed after injected exceptions, so
in theory it would not be a problem to use KVM_INTERRUPT when
an injected exception is present.  However, DOSEMU is using
run->ready_for_interrupt_injection to detect interrupt windows
and then using KVM_SET_SREGS/KVM_SET_REGS to inject the
interrupt manually.  For this to work, the interrupt window
must be delayed after the completion of the previous event
injection.

Cc: stable@vger.kernel.org
Reported-by: Stas Sergeev <stsp2@yandex.ru>
Tested-by: Stas Sergeev <stsp2@yandex.ru>
Fixes: 71cc849b7093 ("KVM: x86: Fix split-irqchip vs interrupt injection window request")
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 043264d97e9ab74cc9661c8b1f9c00c8ce24cad9)
---
 arch/x86/kvm/x86.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4116567f3d44..5e921f1e00db 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4358,8 +4358,18 @@ static int kvm_cpu_accept_dm_intr(struct kvm_vcpu *vcpu)
 
 static int kvm_vcpu_ready_for_interrupt_injection(struct kvm_vcpu *vcpu)
 {
+	/*
+	 * Do not cause an interrupt window exit if an exception
+	 * is pending or an event needs reinjection; userspace
+	 * might want to inject the interrupt manually using KVM_SET_REGS
+	 * or KVM_SET_SREGS.  For that to work, we must be at an
+	 * instruction boundary and with no events half-injected.
+	 */
 	return kvm_arch_interrupt_allowed(vcpu) &&
-		kvm_cpu_accept_dm_intr(vcpu);
+		kvm_cpu_accept_dm_intr(vcpu) &&
+	        !kvm_event_needs_reinjection(vcpu)
+	        !vcpu->arch.exception.pending;
+
 }
 
 static int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
-- 
2.27.0


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

* Re: [PATCH v2] KVM: x86: accept userspace interrupt only if no event is injected
  2021-07-27 17:06 [PATCH v2] KVM: x86: accept userspace interrupt only if no event is injected Paolo Bonzini
@ 2021-07-27 20:58 ` Sean Christopherson
  2021-07-27 21:08   ` Paolo Bonzini
  0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2021-07-27 20:58 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm, stable, Stas Sergeev

On Tue, Jul 27, 2021, Paolo Bonzini wrote:
> Once an exception has been injected, any side effects related to
> the exception (such as setting CR2 or DR6) have been taked place.
> Therefore, once KVM sets the VM-entry interruption information
> field or the AMD EVENTINJ field, the next VM-entry must deliver that
> exception.
> 
> Pending interrupts are processed after injected exceptions, so
> in theory it would not be a problem to use KVM_INTERRUPT when
> an injected exception is present.  However, DOSEMU is using
> run->ready_for_interrupt_injection to detect interrupt windows
> and then using KVM_SET_SREGS/KVM_SET_REGS to inject the
> interrupt manually.  For this to work, the interrupt window
> must be delayed after the completion of the previous event
> injection.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Stas Sergeev <stsp2@yandex.ru>
> Tested-by: Stas Sergeev <stsp2@yandex.ru>
> Fixes: 71cc849b7093 ("KVM: x86: Fix split-irqchip vs interrupt injection window request")
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> (cherry picked from commit 043264d97e9ab74cc9661c8b1f9c00c8ce24cad9)

?  I can't find 043264d97e9ab74cc9661c8b1f9c00c8ce24cad9 anywhere.

> ---
>  arch/x86/kvm/x86.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 4116567f3d44..5e921f1e00db 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4358,8 +4358,18 @@ static int kvm_cpu_accept_dm_intr(struct kvm_vcpu *vcpu)
>  
>  static int kvm_vcpu_ready_for_interrupt_injection(struct kvm_vcpu *vcpu)
>  {
> +	/*
> +	 * Do not cause an interrupt window exit if an exception
> +	 * is pending or an event needs reinjection; userspace
> +	 * might want to inject the interrupt manually using KVM_SET_REGS
> +	 * or KVM_SET_SREGS.  For that to work, we must be at an
> +	 * instruction boundary and with no events half-injected.
> +	 */
>  	return kvm_arch_interrupt_allowed(vcpu) &&
> -		kvm_cpu_accept_dm_intr(vcpu);
> +		kvm_cpu_accept_dm_intr(vcpu) &&

Opportunistically align this indentation?

> +	        !kvm_event_needs_reinjection(vcpu)

Missing &&, apparently the mysterious cherry-pick didn't go so well :-)

> +	        !vcpu->arch.exception.pending;
> +
>  }
>  
>  static int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
> -- 
> 2.27.0
> 

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

* Re: [PATCH v2] KVM: x86: accept userspace interrupt only if no event is injected
  2021-07-27 20:58 ` Sean Christopherson
@ 2021-07-27 21:08   ` Paolo Bonzini
  0 siblings, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2021-07-27 21:08 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: linux-kernel, kvm, stable, Stas Sergeev

On 27/07/21 22:58, Sean Christopherson wrote:
>> ---
>>   arch/x86/kvm/x86.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 4116567f3d44..5e921f1e00db 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -4358,8 +4358,18 @@ static int kvm_cpu_accept_dm_intr(struct kvm_vcpu *vcpu)
>>   
>>   static int kvm_vcpu_ready_for_interrupt_injection(struct kvm_vcpu *vcpu)
>>   {
>> +	/*
>> +	 * Do not cause an interrupt window exit if an exception
>> +	 * is pending or an event needs reinjection; userspace
>> +	 * might want to inject the interrupt manually using KVM_SET_REGS
>> +	 * or KVM_SET_SREGS.  For that to work, we must be at an
>> +	 * instruction boundary and with no events half-injected.
>> +	 */
>>   	return kvm_arch_interrupt_allowed(vcpu) &&
>> -		kvm_cpu_accept_dm_intr(vcpu);
>> +		kvm_cpu_accept_dm_intr(vcpu) &&
> 
> Opportunistically align this indentation?

Yep, good idea.

>> +	        !kvm_event_needs_reinjection(vcpu)
> 
> Missing &&, apparently the mysterious cherry-pick didn't go so well :-)

Well, yeah.  The only way I can excuse myself, is by not being the kind 
of person that yells for such stupid things...

Paolo


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

end of thread, other threads:[~2021-07-27 21:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 17:06 [PATCH v2] KVM: x86: accept userspace interrupt only if no event is injected Paolo Bonzini
2021-07-27 20:58 ` Sean Christopherson
2021-07-27 21:08   ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).