Re: [PATCH 3/3] KVM: Reset PIT irq injection logic when the PIT IRQ is unmasked

[Marcelo Tosatti <mtosatti@redhat.com>]

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.