WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xen: do not unmask disabled IRQ on eoi.
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Fri, 29 Oct 2010 15:32:37 +0100
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, "jeremy@xxxxxxxx" <jeremy@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxxxx>
Delivery-date: Fri, 29 Oct 2010 07:34:30 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <alpine.DEB.2.00.1010221648500.10348@kaball-desktop>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1287139966-19391-2-git-send-email-ian.campbell@xxxxxxxxxx> <alpine.DEB.2.00.1010151614420.2423@kaball-desktop> <1287160335.2003.7973.camel@xxxxxxxxxxxxxxxxxxxxxx> <1287160787.2003.7979.camel@xxxxxxxxxxxxxxxxxxxxxx> <alpine.DEB.2.00.1010151757240.2423@kaball-desktop> <4CBC1CF1020000780001DA3F@xxxxxxxxxxxxxxxxxx> <alpine.DEB.2.00.1010181530200.2423@kaball-desktop> <4CBC80E6020000780001DC78@xxxxxxxxxxxxxxxxxx> <alpine.DEB.2.00.1010181754180.2423@kaball-desktop> <4CBD5883020000780001DEC4@xxxxxxxxxxxxxxxxxx> <alpine.DEB.2.00.1010211411100.10348@kaball-desktop> <4CC15C5B020000780001E8B2@xxxxxxxxxxxxxxxxxx> <1287734836.29224.648.camel@xxxxxxxxxxxxxxxxxxxxx> <4CC16797020000780001E8F7@xxxxxxxxxxxxxxxxxx> <1287736466.11851.5609.camel@xxxxxxxxxxxxxxxxxxxxxx> <alpine.DEB.2.00.1010221244370.10348@kaball-desktop> <4CC1C061020000780001E9F2@xxxxxxxxxxxxxxxxxx> <alpine.DEB.2.00.1010221648500.10348@kaball-desktop>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
I realized that this patch was too complicated so I did some refactoring
and I came up with a simpler patch, easier to understand.

The catch is that I gave up trying to prevent any unwanted notifications
between the eoi hypercall and the following mask_irq, but the current
code has the same problem anyway (actually it is worse because it
doesn't even realize that the irq is supposed to stay masked).
Thus it doesn't introduce any regressions, solves the bug IanC reported
(the virq storm) and improves things for pirqs as well.

---

xen: use do not clear and mask evtchns in __xen_evtchn_do_upcall

Switch virqs and pirqs that don't need EOI (in Xen acktype ==
ACKTYPE_NONE, that means the machine irq is actually edge)
to handle_edge_irq.

Use handle_fasteoi_irq for pirqs that need eoi (they generally
correspond to level triggered irqs), no risk in loosing interrupts
because we have to EOI the irq anyway.

This change has the following benefits:

- it uses the very same handlers that Linux would use on native for the
same irqs;

- it uses these handlers in the same way Linux would use them: it let
Linux mask\unmask and ack the irq when Linux want to mask\unmask and ack
the irq;

However it is obviously not easy to understand and it has to work around
the limitations of PHYSDEVOP_pirq_eoi_gmfn.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

diff --git a/drivers/xen/events.c b/drivers/xen/events.c
index 175e931..a33106c 100644
--- a/drivers/xen/events.c
+++ b/drivers/xen/events.c
@@ -436,21 +436,71 @@ static bool identity_mapped_irq(unsigned irq)
        return irq < get_nr_hw_irqs();
 }
 
-static void pirq_eoi(unsigned int irq)
+static void xen_move_irq(unsigned int irq)
+{
+       struct irq_desc *desc = irq_to_desc(irq);
+       int evtchn = evtchn_from_irq(irq);
+       struct shared_info *s = HYPERVISOR_shared_info;
+
+       if (likely(!(desc->status & IRQ_DISABLED))) {
+               if (unlikely(sync_test_bit(evtchn, &s->evtchn_mask[0])))
+                       move_masked_irq(irq);
+               else
+                       move_native_irq(irq);
+       }
+}
+
+static void eoi_pirq(unsigned int irq)
 {
        struct irq_info *info = info_for_irq(irq);
        struct physdev_eoi eoi = { .irq = info->u.pirq.gsi };
-       bool need_eoi;
+       int evtchn = evtchn_from_irq(irq);
+       int rc = 0;
+       struct irq_desc *desc = irq_to_desc(irq);
 
-       need_eoi = pirq_needs_eoi(irq);
+       if (!VALID_EVTCHN(evtchn))
+               return;
 
-       if (!need_eoi || !pirq_eoi_does_unmask)
-               unmask_evtchn(info->evtchn);
+       xen_move_irq(irq);
 
-       if (need_eoi) {
-               int rc = HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi);
-               WARN_ON(rc);
-       }
+       clear_evtchn(evtchn);
+
+       if (!pirq_needs_eoi(irq))
+               return;
+
+       rc = HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi);
+       WARN_ON(rc);
+
+       /* when PHYSDEVOP_eoi won't automatically unmask the evtchn we won't
+        * need this anymore */
+       if (pirq_eoi_does_unmask && desc->status & IRQ_DISABLED)
+               mask_irq(irq);
+}
+
+static void mask_ack_pirq(unsigned int irq)
+{
+       struct irq_info *info = info_for_irq(irq);
+       struct physdev_eoi eoi = { .irq = info->u.pirq.gsi };
+       int evtchn = evtchn_from_irq(irq);
+       int rc;
+
+       if (!VALID_EVTCHN(evtchn))
+               return;
+
+       mask_irq(irq);
+       move_masked_irq(irq);
+       clear_evtchn(evtchn);
+
+       if (!pirq_needs_eoi(irq))
+               return;
+
+       rc = HYPERVISOR_physdev_op(PHYSDEVOP_eoi, &eoi);
+       WARN_ON(rc);
+
+       /* when PHYSDEVOP_eoi won't automatically unmask the evtchn we won't
+        * need this anymore */
+       if (pirq_eoi_does_unmask)
+               mask_irq(irq);
 }
 
 static void pirq_query_unmask(int irq)
@@ -481,6 +531,7 @@ static bool probing_irq(int irq)
 
 static unsigned int startup_pirq(unsigned int irq)
 {
+       struct irq_desc *desc;
        struct evtchn_bind_pirq bind_pirq;
        struct irq_info *info = info_for_irq(irq);
        int evtchn = evtchn_from_irq(irq);
@@ -510,8 +561,25 @@ static unsigned int startup_pirq(unsigned int irq)
        bind_evtchn_to_cpu(evtchn, 0);
        info->evtchn = evtchn;
 
+       /* If the pirq does not need an eoi than we can use handle_edge_irq
+        * and ack it right away, clearing the evtchn before calling
+        * handle_IRQ_event. If the pirq does need an eoi than we can use
+        * the fasteoi handler without loosing any interrupts because the
+        * physical interrupt will still be waiting for an eoi as well.
+        *
+        * Only after EVTCHNOP_bind_pirq Xen reliably tells us what
+        * kind of pirq this is, so we have to wait until now to make the
+        * choice.
+        * Afterwards Xen might temporarily set the needs_eoi flag for a
+        * particular pirq, but that doesn't affect our choice here that
+        * depends on the nature of the interrupt. */
+       desc = irq_to_desc(irq);
+       if (!pirq_needs_eoi(irq))
+               desc->handle_irq = handle_edge_irq;
+
  out:
-       pirq_eoi(irq);
+       eoi_pirq(irq);
+       unmask_irq(irq);
 
        return 0;
 }
@@ -538,29 +606,6 @@ static void shutdown_pirq(unsigned int irq)
        info->evtchn = 0;
 }
 
-static void ack_pirq(unsigned int irq)
-{
-       move_masked_irq(irq);
-       
-       pirq_eoi(irq);
-}
-
-static void end_pirq(unsigned int irq)
-{
-       int evtchn = evtchn_from_irq(irq);
-       struct irq_desc *desc = irq_to_desc(irq);
-
-       if (WARN_ON(!desc))
-               return;
-
-       if ((desc->status & (IRQ_DISABLED|IRQ_PENDING)) ==
-           (IRQ_DISABLED|IRQ_PENDING)) {
-               shutdown_pirq(irq);
-       } else if (VALID_EVTCHN(evtchn)) {
-               pirq_eoi(irq);
-       }
-}
-
 static int find_irq_by_gsi(unsigned gsi)
 {
        int irq;
@@ -750,7 +795,7 @@ int bind_evtchn_to_irq(unsigned int evtchn)
                irq = find_unbound_irq();
 
                set_irq_chip_and_handler_name(irq, &xen_dynamic_chip,
-                                             handle_fasteoi_irq, "event");
+                                             handle_edge_irq, "event");
 
                evtchn_to_irq[evtchn] = irq;
                irq_info[irq] = mk_evtchn_info(evtchn);
@@ -1074,9 +1119,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)
@@ -1199,10 +1241,17 @@ static void ack_dynirq(unsigned int irq)
 {
        int evtchn = evtchn_from_irq(irq);
 
-       move_masked_irq(irq);
+       if (!VALID_EVTCHN(evtchn))
+               return;
 
-       if (VALID_EVTCHN(evtchn))
-               unmask_evtchn(evtchn);
+       xen_move_irq(irq);
+       clear_evtchn(evtchn);
+}
+
+static void mask_ack_dynirq(unsigned int irq)
+{
+       mask_irq(irq);
+       ack_dynirq(irq);
 }
 
 static int retrigger_irq(unsigned int irq)
@@ -1384,11 +1433,11 @@ 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,
 
-       .eoi            = ack_dynirq,
+       .ack            = ack_dynirq,
+       .mask_ack       = mask_ack_dynirq,
        .set_affinity   = set_affinity_irq,
        .retrigger      = retrigger_irq,
 };
@@ -1409,14 +1458,12 @@ static struct irq_chip xen_pirq_chip __read_mostly = {
        .startup        = startup_pirq,
        .shutdown       = shutdown_pirq,
 
-       .enable         = pirq_eoi,
-       .unmask         = unmask_irq,
-
-       .disable        = mask_irq,
        .mask           = mask_irq,
+       .unmask         = unmask_irq,
 
-       .eoi            = ack_pirq,
-       .end            = end_pirq,
+       .ack            = eoi_pirq,
+       .eoi            = eoi_pirq,
+       .mask_ack       = mask_ack_pirq,
 
        .set_affinity   = set_affinity_irq,
 

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