[v5,4/4] KVM: x86: Add support for local interrupt requests from userspace

Commit Message

Paolo Bonzini July 28, 2015, 10:05 p.m. UTC

On 28/07/2015 21:06, Steve Rutherford wrote:
>>> > > +		if (!kvm_run->ready_for_interrupt_injection &&
>>> > > +		    ready_for_interrupt_injection)
>>> > > +			kvm_make_request(KVM_REQ_PIC_UNMASK_EXIT, vcpu);
>>> > > +
>>> > > +		kvm_run->ready_for_interrupt_injection =
>>> > > +				ready_for_interrupt_injection;
>>> > > +	} else {
>>> > >  		kvm_run->ready_for_interrupt_injection =
>>> > >  			kvm_arch_interrupt_allowed(vcpu) &&
>>> > >  			!kvm_cpu_has_interrupt(vcpu) &&
>>> > >  			!kvm_event_needs_reinjection(vcpu);
>>> > > +	}
>>> > >  }
>>> > >  
>>> > >  static void update_cr8_intercept(struct kvm_vcpu *vcpu)
>> > 
>> > Why is this necessary?  Could it just set
>> > kvm_run->ready_for_interrupt_injection as in the pic_in_kernel case?
> The goal is to couple the interrupt ack cycle as closely as possible
> with the injection of the local interrupt (which occur more or less
> atomically on real hardware). The idea is to only ever attempt to
> inject local interrupts when the CPU/APIC is ready to immediately
> accept. 

Ok, I understand it now.  However, you're still not causing an exit 
when LVT0 changes, are you?  post_kvm_run_save is not run until the
next exit to userspace, which could be a long time later.

So, I think that you do not need KVM_REQ_PIC_UNMASK_EXIT.  Instead,
you can modify dm_request_for_irq_injection to handle the split-irqchip
case, like this:

	if (!vcpu->run->request_interrupt_window || pic_in_kernel(vcpu->kvm))
		return false;

	if (kvm_cpu_has_interrupt(vcpu))
		return false;

        return (irqchip_split(vcpu->kvm)
                ? kvm_apic_accept_pic_intr(vcpu)
		: kvm_arch_interrupt_allowed(vcpu));

This will cause KVM_RUN to return -EINTR, which QEMU happens to handle
the same way as KVM_EXIT_IRQ_WINDOW_OPEN.  If you prefer the explicit
reason, this small change will provide it:


Feel free to post v6 of this patch only.  Everything else is mostly
okay; there are some leftovers here and there (lapic_in_kernel,
GET_VECTOR_FROM_USERSPACE) but I can fix that.

How is the integration with QEMU going?  With this latest iteration
it should be relatively easy.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Steve Rutherford July 29, 2015, 12:50 a.m. UTC | #1

On Wed, Jul 29, 2015 at 12:05:37AM +0200, Paolo Bonzini wrote:
> Ok, I understand it now.  However, you're still not causing an exit 
> when LVT0 changes, are you?  post_kvm_run_save is not run until the
> next exit to userspace, which could be a long time later.

Yes! This is definitely right. This may wait as long as an entire entry/exit
before actually exiting to userspace. Moving into dm_request_for_irq_injection
is definitely the way to go.
> 
> So, I think that you do not need KVM_REQ_PIC_UNMASK_EXIT.  Instead,
> you can modify dm_request_for_irq_injection to handle the split-irqchip
> case, like this:
> 
> 	if (!vcpu->run->request_interrupt_window || pic_in_kernel(vcpu->kvm))
> 		return false;
> 
> 	if (kvm_cpu_has_interrupt(vcpu))
> 		return false;
> 
>         return (irqchip_split(vcpu->kvm)
>                 ? kvm_apic_accept_pic_intr(vcpu)
> 		: kvm_arch_interrupt_allowed(vcpu));
> 
> This will cause KVM_RUN to return -EINTR, which QEMU happens to handle
> the same way as KVM_EXIT_IRQ_WINDOW_OPEN.
I definitely prefer the explit exit reason.
It's also a bit easier to make work with our VMM ;)

> Feel free to post v6 of this patch only.  Everything else is mostly
> okay; there are some leftovers here and there (lapic_in_kernel,
> GET_VECTOR_FROM_USERSPACE) but I can fix that.
I'll give it another once over to remove the dead code. Sorry about
leaving that junk in.

> How is the integration with QEMU going?  With this latest iteration
> it should be relatively easy.
A new team member is sinking his teeth into it, as an starter project. 
He'll likely have a prototype of it working soon. 

Steve
> 
> Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 5ef2560075bf..3269169233fb 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6720,8 +6720,8 @@  static int vcpu_run(struct kvm_vcpu *vcpu)
 			kvm_inject_pending_timer_irqs(vcpu);
 
 		if (dm_request_for_irq_injection(vcpu)) {
-			r = -EINTR;
-			vcpu->run->exit_reason = KVM_EXIT_INTR;
+			r = 0;
+			vcpu->run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
 			++vcpu->stat.request_irq_exits;
 			break;
 		}