xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Campbell <ian.campbell@citrix.com>
To: xen-devel@lists.xen.org
Cc: Ian Campbell <ian.campbell@citrix.com>,
	stefano.stabellini@eu.citrix.com, julien.grall@linaro.org,
	tim@xen.org, george.dunlap@citrix.com,
	Oleksandr Tyshchenko <oleksandr.tyshchenko@globallogic.com>
Subject: [PATCH] xen: arm: increase priority of SGIs used as IPIs
Date: Tue, 28 Jan 2014 16:51:18 +0000	[thread overview]
Message-ID: <1390927878-7048-1-git-send-email-ian.campbell@citrix.com> (raw)

Code such as on_selected_cpus expects/requires that an IPI can preempt a
processor which is just handling a normal interrupt. Lacking this property can
result in a deadlock between two CPUs trying to IPI each other from interrupt
context.

For the time being there is only two priorities, IRQ and IPI, although it is
also conceivable that in the future some IPIs might be higher priority than
others. This could be used to implement a better BUG() than we have now, but I
haven't tackled that yet.

Tested with a debug patch which sends a local IPI from a keyhandler, which is
run in serial interrupt context.

This should also fix the issue reported by Oleksandr in "xen/arm:
maintenance_interrupt SMP fix" without resorting to trylock.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Oleksandr Tyshchenko <oleksandr.tyshchenko@globallogic.com>
---
I think this is probably 4.5 material at this point.

Tested with "HACK: dump pcpu state keyhandler" which I'll post for
completeness. It gives:
(XEN) Xen call trace:
(XEN)    [<0000000000212048>] dump_pcpus+0x28/0x2c (PC)
(XEN)    [<000000000021256c>] handle_keypress+0x70/0xb0 (LR)
(XEN)    [<000000000023ed00>] __serial_rx+0x20/0x6c
(XEN)    [<000000000023f8ac>] serial_rx+0xb4/0xc4
(XEN)    [<00000000002409ec>] serial_rx_interrupt+0xb0/0xd4
(XEN)    [<00000000002404b4>] ns16550_interrupt+0x6c/0x90
(XEN)    [<0000000000245fc0>] do_IRQ+0x144/0x1b4
(XEN)    [<0000000000245a28>] gic_interrupt+0x60/0xf8
(XEN)    [<000000000024be64>] do_trap_irq+0x10/0x18
(XEN)    [<000000000024e240>] hyp_irq+0x5c/0x60
(XEN)    [<0000000000249324>] init_done+0x10/0x18
(XEN)    [<0000000000000080>] 0000000000000080
---
 xen/arch/arm/gic.c        |   19 +++++++++++++------
 xen/arch/arm/time.c       |    6 +++---
 xen/include/asm-arm/gic.h |   22 ++++++++++++++++++++++
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index dcf9cd4..ee37019 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -319,7 +319,8 @@ static void __init gic_dist_init(void)
 
     /* Default priority for global interrupts */
     for ( i = 32; i < gic.lines; i += 4 )
-        GICD[GICD_IPRIORITYR + i / 4] = 0xa0a0a0a0;
+        GICD[GICD_IPRIORITYR + i / 4] =
+            GIC_PRI_IRQ<<24 | GIC_PRI_IRQ<<16 | GIC_PRI_IRQ<<8 | GIC_PRI_IRQ;
 
     /* Disable all global interrupts */
     for ( i = 32; i < gic.lines; i += 32 )
@@ -341,8 +342,12 @@ static void __cpuinit gic_cpu_init(void)
     GICD[GICD_ICENABLER] = 0xffff0000; /* Disable all PPI */
     GICD[GICD_ISENABLER] = 0x0000ffff; /* Enable all SGI */
     /* Set PPI and SGI priorities */
-    for (i = 0; i < 32; i += 4)
-        GICD[GICD_IPRIORITYR + i / 4] = 0xa0a0a0a0;
+    for (i = 0; i < 16; i += 4)
+        GICD[GICD_IPRIORITYR + i / 4] =
+            GIC_PRI_IPI<<24 | GIC_PRI_IPI<<16 | GIC_PRI_IPI<<8 | GIC_PRI_IPI;
+    for (i = 16; i < 32; i += 4)
+        GICD[GICD_IPRIORITYR + i / 4] =
+            GIC_PRI_IRQ<<24 | GIC_PRI_IRQ<<16 | GIC_PRI_IRQ<<8 | GIC_PRI_IRQ;
 
     /* Local settings: interface controller */
     GICC[GICC_PMR] = 0xff;                /* Don't mask by priority */
@@ -538,7 +543,8 @@ void gic_disable_cpu(void)
 void gic_route_ppis(void)
 {
     /* GIC maintenance */
-    gic_route_dt_irq(&gic.maintenance, cpumask_of(smp_processor_id()), 0xa0);
+    gic_route_dt_irq(&gic.maintenance, cpumask_of(smp_processor_id()),
+                     GIC_PRI_IRQ);
     /* Route timer interrupt */
     route_timer_interrupt();
 }
@@ -553,7 +559,8 @@ void gic_route_spis(void)
         if ( (irq = serial_dt_irq(seridx)) == NULL )
             continue;
 
-        gic_route_dt_irq(irq, cpumask_of(smp_processor_id()), 0xa0);
+        gic_route_dt_irq(irq, cpumask_of(smp_processor_id()),
+                         GIC_PRI_IRQ);
     }
 }
 
@@ -777,7 +784,7 @@ int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq,
     level = dt_irq_is_level_triggered(irq);
 
     gic_set_irq_properties(irq->irq, level, cpumask_of(smp_processor_id()),
-                           0xa0);
+                           GIC_PRI_IRQ);
 
     retval = __setup_irq(desc, irq->irq, action);
     if (retval) {
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 81e3e28..68b939d 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -222,11 +222,11 @@ static void vtimer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
 void __cpuinit route_timer_interrupt(void)
 {
     gic_route_dt_irq(&timer_irq[TIMER_PHYS_NONSECURE_PPI],
-                     cpumask_of(smp_processor_id()), 0xa0);
+                     cpumask_of(smp_processor_id()), GIC_PRI_IRQ);
     gic_route_dt_irq(&timer_irq[TIMER_HYP_PPI],
-                     cpumask_of(smp_processor_id()), 0xa0);
+                     cpumask_of(smp_processor_id()), GIC_PRI_IRQ);
     gic_route_dt_irq(&timer_irq[TIMER_VIRT_PPI],
-                     cpumask_of(smp_processor_id()), 0xa0);
+                     cpumask_of(smp_processor_id()), GIC_PRI_IRQ);
 }
 
 /* Set up the timer interrupt on this CPU */
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 9c6f9bb..25b2b24 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -129,6 +129,28 @@
 #define GICH_LR_CPUID_SHIFT     9
 #define GICH_VTR_NRLRGS         0x3f
 
+/*
+ * The minimum GICC_BPR is required to be in the range 0-3. We set
+ * GICC_BPR to 0 but we must expect that it might be 3. This means we
+ * can rely on premption between the following ranges:
+ * 0xf0..0xff
+ * 0xe0..0xdf
+ * 0xc0..0xcf
+ * 0xb0..0xbf
+ * 0xa0..0xaf
+ * 0x90..0x9f
+ * 0x80..0x8f
+ *
+ * Priorities within a range will not preempt each other.
+ *
+ * A GIC must support a mimimum of 16 priority levels.
+ */
+#define GIC_PRI_LOWEST     0xf0
+#define GIC_PRI_IRQ        0xa0
+#define GIC_PRI_IPI        0x90 /* IPIs must preempt normal interrupts */
+#define GIC_PRI_HIGHEST    0x80 /* Higher priorities belong to Secure-World */
+
+
 #ifndef __ASSEMBLY__
 #include <xen/device_tree.h>
 
-- 
1.7.10.4

             reply	other threads:[~2014-01-28 16:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-28 16:51 Ian Campbell [this message]
2014-01-28 16:51 ` [PATCH] xen: arm: increase priority of SGIs used as IPIs Ian Campbell
2014-01-28 17:50 ` Stefano Stabellini
2014-01-28 18:20   ` Oleksandr Tyshchenko
2014-03-13 12:26 ` Ian Campbell
2014-03-13 12:33 ` Julien Grall
2014-03-13 12:45   ` Ian Campbell
2014-03-13 12:56     ` Julien Grall
2014-03-13 13:35       ` Oleksandr Tyshchenko
2014-03-16 16:38         ` Stefano Stabellini
2014-03-17 10:02           ` Ian Campbell
2014-03-17 11:29             ` Ian Campbell
2014-03-17 11:34               ` Julien Grall
2014-03-17 11:36                 ` Ian Campbell
2014-03-17 12:42                   ` Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1390927878-7048-1-git-send-email-ian.campbell@citrix.com \
    --to=ian.campbell@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=julien.grall@linaro.org \
    --cc=oleksandr.tyshchenko@globallogic.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).