Re: [Xen-devel] [PATCH] xen: do not unmask disabled IRQ on eoi.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 15 Oct 2010, Ian Campbell wrote:
> This prevents a guest from being able to trigger an interrupt storm in
> dom0 rendering it inoperable. In particular this effects event
> channels delivered to userspace by the /dev/xen/evtchn driver which
> uses disable_irq in its interupt handler to prevent further interrupts
> firing until the interrupt has been handled by the userspace
> application and it has requested an unmask. Without this patch the
> event channel is re-enabled immediately after the interrupt handler
> completes.
> 
> The issue was introduced by 0672fb44a111 "xen/events: change to using
> fasteoi".
> 
> In the specific instance I saw a domU would spin sending console event
> channel notifications to dom0 because its console ring was full (this
> behaviour has since been tempered by f3483182666a "xen/hvc: only
> notify if we actually sent something") but xenconsoled would be unable
> to run due to the storm and hence the ring would never be drained.
> 
> I'm not convinced this is the right way to go about this -- there does
> not seem to be much precedent and I would have expected some sort of
> generic handling but I cannot see any.
> 


Reading a little bit more about the fasteoi handler, it seems to me that
a better solution would be not to mask_evtchn and clear_evtchn in
__xen_evtchn_do_upcall, but just clear_evtchn in the eoi handler.
This would also be much more similar to the way the fasteoi handler is
used by the ioapic chip.
The appended patch has been only smoked tested.


diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 175e931..6de1528 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -1074,9 +1074,6 @@ static void __xen_evtchn_do_upcall(struct pt_regs *regs)
                                int irq = evtchn_to_irq[port];
                                struct irq_desc *desc;
 
-                               mask_evtchn(port);
-                               clear_evtchn(port);
-
                                if (irq != -1) {
                                        desc = irq_to_desc(irq);
                                        if (desc)
@@ -1202,7 +1199,7 @@ static void ack_dynirq(unsigned int irq)
        move_masked_irq(irq);
 
        if (VALID_EVTCHN(evtchn))
-               unmask_evtchn(evtchn);
+               clear_evtchn(evtchn);
 }
 
 static int retrigger_irq(unsigned int irq)
@@ -1384,7 +1381,6 @@ void xen_irq_resume(void)
 static struct irq_chip xen_dynamic_chip __read_mostly = {
        .name           = "xen-dyn",
 
-       .disable        = mask_irq,
        .mask           = mask_irq,
        .unmask         = unmask_irq,
 

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