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

Sheng Yang
2008-12-26T01:53:14Z

On Thursday 25 December 2008 21:26:29 Avi Kivity wrote:
> 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?

Well, I just think using fifo is more generic and unify the logic of three
type of interrupt easily, something seems more elegant.

> >>> @@ -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.

OK. I would give a bitmap to kvm struct with gsi_msg which is unable to
overflow.

--
regards
Yang, Sheng