Re: [PATCH 1/4] KVM: Using kfifo for irq recording

Avi Kivity
2008-12-25T13:26:33Z

Sheng Yang wrote:
> On Thursday 25 December 2008 19:07:22 Avi Kivity wrote:
>
>> Sheng Yang wrote:
>>
>>> For MSI-X, we have to deal with multiply IRQ with same IRQ handler, so
>>> it's necessary to record the IRQ that trigger the IRQ handler.
>>>
>> Does MSI-X disallowing coalescing two requests into one interrupt? Or
>> can we still coalesce interrupts (perhaps by recording them as a (irq,
>> cpu) pair?)
>>
>
> Disallow? Not quite understand. PCI spec said OS don't need to ensure the
> sequence they handled is the same as they happened. This struct is used just
> because we lost information of irq after schedule_work...
>
>

Why can't we store this information in a bitmap? There are a limited
number of irqs.

The only reason I can think of for using a fifo is if we want to
preserve the number and ordering of interrupts. Is there another reason?

>>> @@ -313,6 +314,9 @@ struct kvm_assigned_dev_kernel {
>>> int host_irq;
>>> bool host_irq_disabled;
>>> int guest_irq;
>>> +#define KVM_ASSIGNED_DEV_IRQ_FIFO_LEN 0x100
>>> + struct kfifo *irq_fifo;
>>> + spinlock_t irq_fifo_lock;
>>> #define KVM_ASSIGNED_DEV_GUEST_INTX (1 << 0)
>>>
>> What if it runs out?
>>
>> What does real hardware do? I'm sure it doesn't have a 100-entry queue.
>>
>
> 0x100 is just a simple number which I thought different interrupts of same
> MSI-X device can happen at same period(indeed it's 0x100/sizeof(int)). Maybe
> not that many. And it just used by work function later to find what guest
> vector is, and then inject the correlated interrupt to the guest.
>

Maybe it's better to do the conversion immediately, so we can store the
information in a structure that's not prone to overflow.

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