From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH 3/3] KVM: Reset PIT irq injection logic when the PIT IRQ
	is unmasked
Date: Mon, 5 Jan 2009 19:58:50 -0200
Message-ID: <20090105215850.GA22009@amt.cnet>
References: <1231085685-32201-1-git-send-email-avi@redhat.com> <1231085685-32201-4-git-send-email-avi@redhat.com> <20090105183159.GC5592@amt.cnet> <49627495.9020203@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Sheng Yang <sheng@linux.intel.com>, kvm@vger.kernel.org
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:37195 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753528AbZAEV7J (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 5 Jan 2009 16:59:09 -0500
Content-Disposition: inline
In-Reply-To: <49627495.9020203@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Jan 05, 2009 at 10:59:01PM +0200, Avi Kivity wrote:
> Marcelo Tosatti wrote:
>> On Sun, Jan 04, 2009 at 06:14:45PM +0200, Avi Kivity wrote:
>>   
>>> While the PIT is masked the guest cannot ack the irq, so the reinject logic
>>> will never allow the interrupt to be injected.
>>>
>>> Fix by resetting the reinjection counters on unmask.
>>>
>>> Unbreaks Xen.
>>>
>>> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
>>> index 528daad..d78d430 100644
>>> --- a/arch/x86/kvm/i8254.c
>>> +++ b/arch/x86/kvm/i8254.c
>>> @@ -539,6 +539,16 @@ void kvm_pit_reset(struct kvm_pit *pit)
>>>  	pit->pit_state.irq_ack = 1;
>>>  }
>>>  +static void pit_mask_notifer(struct kvm_irq_mask_notifier *kimn, 
>>> int mask)
>>> +{
>>> +	struct kvm_pit *pit = container_of(kimn, struct kvm_pit, mask_notifier);
>>> +
>>> +	if (!mask) {
>>> +		atomic_set(&pit->pit_state.pit_timer.pending, 0);
>>> +		pit->pit_state.irq_ack = 1;
>>> +	}
>>> +}
>>>     
>>
>> I'm not sure about zeroing the counter here. The guest can mask the
>> interrupt during normal operation, and in such cases you want the
>> pending count to be retained (and reinjected later).
>>
>> I suppose setting irq_ack to one is enough.
>>   
>
> I'm worried about:
>
> - boot guest using local apic timer
> - reset
> - boot with pit timer
> - a zillion interrupts
>
> So at the very least, we need a limiter.

Or have a new notifier on kvm_pic_reset, instead of simply acking one
pending irq? That seems the appropriate place to zero the counter.