From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@qumranet.com>
Subject: Re: KVM: fix i8259 reset irq acking
Date: Wed, 13 Aug 2008 13:40:34 +0300
Message-ID: <48A2BA22.70805@qumranet.com>
References: <20080731171556.GA2644@dmt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm-devel <kvm@vger.kernel.org>
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from il.qumranet.com ([212.179.150.194]:22142 "EHLO il.qumranet.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752583AbYHMKkf (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 13 Aug 2008 06:40:35 -0400
In-Reply-To: <20080731171556.GA2644@dmt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Marcelo Tosatti wrote:
> The irq ack during pic reset has three problems:
>
> - Ignores slave/master PIC, using gsi 0-8 for both.
> - Generates an ACK even if the APIC is in control.
> - Depends upon IMR being clear, which is broken if the 
> irq was masked at the time it was generated.
>
> The last one causes the BIOS to hang after the first reboot of Windows
> installation, since PIT interrupts stop.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
>
> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> index de70499..2b42d48 100644
> --- a/arch/x86/kvm/i8259.c
> +++ b/arch/x86/kvm/i8259.c
> @@ -195,14 +195,18 @@ int kvm_pic_read_irq(struct kvm *kvm)
>  
>  void kvm_pic_reset(struct kvm_kpic_state *s)
>  {
> -	int irq;
> +	int irq, irqbase;
>  	struct kvm *kvm = s->pics_state->irq_request_opaque;
>  
> -	for (irq = 0; irq < PIC_NUM_PINS; irq++) {
> -		if (!(s->imr & (1 << irq)) && (s->irr & (1 << irq) ||
> -		    s->isr & (1 << irq)))
> -			kvm_notify_acked_irq(kvm, irq);
> -	}
> +	if (s == &s->pics_state->pics[0])
> +		irqbase = 0;
> +	else
> +		irqbase = 8;
> +
> +	if (!kvm_lapic_enabled(kvm->vcpus[0]))
>   

What if the lapic is enabled and passes through PIC interrupts in EXTINT 
mode?

btw, this will oops if we have no vcpu 0.

> +		for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
> +			if (s->irr & (1 << irq) || s->isr & (1 << irq))
> +				kvm_notify_acked_irq(kvm, irq+irqbase);
>   

Yeah.

-- 
error compiling committee.c: too many arguments to function