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

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Fri, 2010-10-15 at 17:12 +0100, Stefano Stabellini wrote:
> 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.

Fails to boot dom0 for me.
        
        irq 20: nobody cared (try booting with the "irqpoll" option)
        Pid: 0, comm: swapper Not tainted 
2.6.32.24-x86_32p-xen0-01025-g5238c00-dirty #205
        Call Trace:
         [<c1377034>] ? printk+0x18/0x1c
         [<c106f5c7>] __report_bad_irq+0x27/0x90
         [<c106f78e>] note_interrupt+0x15e/0x1a0
         [<c1169669>] ? _raw_spin_unlock+0x89/0xa0
         [<c106febc>] handle_fasteoi_irq+0xac/0xd0
         [<c11a6a5f>] __xen_evtchn_do_upcall+0x1cf/0x1f0
         [<c11a6ab8>] xen_evtchn_do_upcall+0x28/0x40
         [<c100b767>] xen_do_upcall+0x7/0xc
         [<c10013a7>] ? hypercall_page+0x3a7/0x1010
         [<c1007472>] ? xen_safe_halt+0x12/0x30
         [<c100397f>] xen_idle+0x5f/0x80
         [<c1009c49>] cpu_idle+0x49/0xa0
         [<c1007c40>] ? xen_save_fl_direct+0x0/0xd
         [<c135d913>] rest_init+0x53/0x60
         [<c1543975>] start_kernel+0x382/0x39f
         [<c15433be>] ? unknown_bootoption+0x0/0x1e4
         [<c154308f>] i386_start_kernel+0x7e/0xa8
         [<c1546471>] xen_start_kernel+0x561/0x5d5
        handlers:
        [<c1223500>] (ata_sff_interrupt+0x0/0xd0)
        [<c128b2b0>] (usb_hcd_irq+0x0/0xe0)
        Disabling IRQ #20
        
I think because you missed the pirq case.

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 175e931..1144489 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -444,8 +444,8 @@ static void pirq_eoi(unsigned int irq)
 
        need_eoi = pirq_needs_eoi(irq);
 
-       if (!need_eoi || !pirq_eoi_does_unmask)
-               unmask_evtchn(info->evtchn);
+       if (need_eoi && pirq_eoi_does_unmask)
+               mask_evtchn(info->evtchn);
+
+       clear_evtchn(info->evtchn);
 
        if (need_eoi) {
                int rc = HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi);
@@ -1052,6 +1056,7 @@ static void __xen_evtchn_do_upcall(struct pt_regs *regs)
 
        do {
                unsigned long pending_words;
+               int irq;
 
                vcpu_info->evtchn_upcall_pending = 0;
 
@@ -1062,6 +1067,24 @@ static void __xen_evtchn_do_upcall(struct pt_regs *regs)
                /* Clear master flag /before/ clearing selector flag. */
                wmb();
 #endif
+
+               /*
+                * Handle timer interrupts before all others, so that all
+                * hardirq handlers see an up-to-date system time even if we
+                * have just woken from a long idle period.
+                */
+               irq = percpu_read(virq_to_irq[VIRQ_TIMER]);
+               if (irq != -1) {
+                       int word_idx;
+                       int bit_idx;
+                       int port = evtchn_from_irq(irq);
+                       word_idx = port / BITS_PER_LONG;
+                       bit_idx = port % BITS_PER_LONG;
+                       if (VALID_EVTCHN(port) &&
+                           (active_evtchns(cpu, s, word_idx) & (1UL<<bit_idx)))
+                               (void)handle_irq(irq, regs);
+               }
+
                pending_words = xchg(&vcpu_info->evtchn_pending_sel, 0);
                while (pending_words != 0) {
                        unsigned long pending_bits;
@@ -1071,11 +1094,9 @@ static void __xen_evtchn_do_upcall(struct pt_regs *regs)
                        while ((pending_bits = active_evtchns(cpu, s, 
word_idx)) != 0) {
                                int bit_idx = __ffs(pending_bits);
                                int port = (word_idx * BITS_PER_LONG) + bit_idx;
-                               int irq = evtchn_to_irq[port];
                                struct irq_desc *desc;
 
-                               mask_evtchn(port);
-                               clear_evtchn(port);
+                               irq = evtchn_to_irq[port];
 
                                if (irq != -1) {
                                        desc = irq_to_desc(irq);
@@ -1202,7 +1223,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 +1405,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,
 
@@ -1412,7 +1432,7 @@ static struct irq_chip xen_pirq_chip __read_mostly = {
        .enable         = pirq_eoi,
        .unmask         = unmask_irq,
 
-       .disable        = mask_irq,
+       //.disable      = mask_irq,
        .mask           = mask_irq,
 
        .eoi            = ack_pirq,



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