[Xen-devel] RE: [PATCH] Handle MSI irq storm

["Shan, Haitao" <haitao.shan@xxxxxxxxx>]

Hi, Keir,

As I was writing the new patch, I had some other considerations.
Concerning PV guests, we can depend on the event-channel-pending to decide 
whether we will set timer and rely on the timer to enable this vector some time 
later.
So for HVM guest, can we also rely on the timer to enable this interrupt vector 
later? (so in hvm_eoi_function, we only unmark the IRQ_INPROGRESS without 
enable it.) Thus, the handling can be the same for both PV guests and HVM 
guests.
How do you think of this scheme? 

Shan Haitao

-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx] 
Sent: 2008年7月3日 17:12
To: Shan, Haitao
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH] Handle MSI irq storm

On 3/7/08 09:56, "Shan, Haitao" <haitao.shan@xxxxxxxxx> wrote:

> For HVM guest, I assume that when EOI (virtual) is written guest has finished
> the interrupt handling (or at least, it has started handling).
> But for non-HVM guest, I have not thought of a good point now. I just assume
> the interrupt has been services as soon as pirq is sent to guest. Do you have
> any suggestions on this?

Yes, you can check whether send_guest_pirq() found the event-channel-pending
flag already set. If so, and the interrupt is ACKTYPE_NONE, then it's a
spurious interrupt.

I would arrange the logic something like as follows:
 1. Push your INPROGRESS logic inside of hvm_do_IRQ_dpci() (you'll have to
create new flag in place of using IRQ_INPROGRESS, obviously). Have
hvm_do_IRQ_dpci() return one of three statuses: not handled, handled, and
already pending. Don't bother clearing the flag in the hvm eoi function.
It's not really a requirement for this scheme to work.
 2. Have send_guest_pirq() return a boolean indicating whether the
event-channel is already pending.
 3. In __do_IRQ_guest() collate already-pending status for each iteration of
the for loop. After the loop, if all receivers indicated already-pending
then disable_irq(), set IRQ_INPROGRESS, set the timer, etc.

Oh, allocate the irq timers outside of irq_desc (i.e., have a separate array
allocated statically in irq.c). Dynamically allocating them on the interrupt
path is not really very great. And remember to kill_timer() if all guest
unbind from the interrupt.

Does that all make sense?

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel