Re: [PATCH v7 4/4] KVM: x86: Add support for local interrupt requests from userspace

[Paolo Bonzini <pbonzini@redhat.com>]

On 30/07/2015 08:21, Steve Rutherford wrote:
>   */
>  int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v)
>  {
> -	if (!irqchip_in_kernel(v->kvm))
> +	if (!pic_in_kernel(v->kvm))
>  		return v->arch.interrupt.pending;
>  
>  	if (kvm_cpu_has_extint(v))
> @@ -75,7 +88,7 @@ int kvm_cpu_has_injectable_intr(struct kvm_vcpu *v)
>   */
>  int kvm_cpu_has_interrupt(struct kvm_vcpu *v)
>  {
> -	if (!irqchip_in_kernel(v->kvm))
> +	if (!pic_in_kernel(v->kvm))
>  		return v->arch.interrupt.pending;
>  
>  	if (kvm_cpu_has_extint(v))
> @@ -103,7 +123,7 @@ int kvm_cpu_get_interrupt(struct kvm_vcpu *v)
>  {
>  	int vector;
>  
> -	if (!irqchip_in_kernel(v->kvm))
> +	if (!pic_in_kernel(v->kvm) && v->arch.interrupt.pending)
>  		return v->arch.interrupt.nr;
>  
>  	vector = kvm_cpu_get_extint(v);

I have one more doubt about these three hunks.

v->arch.interrupt should not be used at all with split irqchip.  In 
particular:

- kvm_cpu_has_injectable_intr should go through kvm_cpu_has_extint and 
query pending_userspace_extint

- same for kvm_cpu_has_interrupt

- kvm_cpu_get_interrupt should go through kvm_cpu_get_extint and 
return/clear v->arch.pending_external_vector.

So I think !irqchip_in_kernel(v->kvm) is the right test.  In 
particular, with pic_in_kernel, kvm_cpu_has_extint's irqchip_split case 
is dead.  I am then not sure how you reach this code in x86.c:

	/* kvm_cpu_has_injectable_intr doesn't take extints into account? */
        } else if (kvm_cpu_has_injectable_intr(vcpu)) {
                /*
                 * Because interrupts can be injected asynchronously, we are
                 * calling check_nested_events again here to avoid a race condition.
                 * See https://lkml.org/lkml/2014/7/2/60 for discussion about this
                 * proposal and current concerns.  Perhaps we should be setting
                 * KVM_REQ_EVENT only on certain events and not unconditionally?
                 */
                if (is_guest_mode(vcpu) && kvm_x86_ops->check_nested_events) {
                        r = kvm_x86_ops->check_nested_events(vcpu, req_int_win);
                        if (r != 0)
                                return r;
                }
                if (kvm_x86_ops->interrupt_allowed(vcpu)) {
			/*
			 * kvm_cpu_get_interrupt does take extints into account
			 * because of the " && v->arch.interrupt.pending", but
			 * you won't get here unless you have an APIC interrupt
			 * pending!
			 */
                        kvm_queue_interrupt(vcpu, kvm_cpu_get_interrupt(vcpu),
                                            false);
                        kvm_x86_ops->set_irq(vcpu);
                }
        }

Paolo