[PATCH] xen: arm: increase priority of SGIs used as IPIs

[PATCH] xen: arm: increase priority of SGIs used as IPIs

[Ian Campbell]

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

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Ian Campbell]

On Tue, 2014-01-28 at 16:51 +0000, Ian Campbell wrote:
> Tested with "HACK: dump pcpu state keyhandler" which I'll post for
> completeness. 

>From 7975fb2a9a27e738d3b551fa2258d65a8c4b0d9a Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Tue, 28 Jan 2014 15:57:33 +0000
Subject: [PATCH] HACK: dump pcpu state keyhandler

---
 xen/arch/arm/gic.c        |    3 +++
 xen/common/keyhandler.c   |   20 ++++++++++++++++++++
 xen/include/asm-arm/gic.h |    1 +
 3 files changed, 24 insertions(+)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index e6257a7..dcf9cd4 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -811,6 +811,9 @@ static void do_sgi(struct cpu_user_regs *regs, int othercpu, enum gic_sgi sgi)
     case GIC_SGI_CALL_FUNCTION:
         smp_call_function_interrupt();
         break;
+    case GIC_SGI_DUMP_HOST_STATE:
+        show_execution_state(regs);
+        break;
     default:
         panic("Unhandled SGI %d on CPU%d", sgi, smp_processor_id());
         break;
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 8e4b3f8..52f0916 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -20,6 +20,7 @@
 #include <xen/init.h>
 #include <asm/debugger.h>
 #include <asm/div64.h>
+#include <asm/gic.h>
 
 static struct keyhandler *key_table[256];
 static unsigned char keypress_key;
@@ -149,6 +150,24 @@ static struct keyhandler dump_registers_keyhandler = {
     .desc = "dump registers"
 };
 
+static void dump_pcpus(unsigned char key, struct cpu_user_regs *regs)
+{
+    printk("'%c' pressed -> dumping PCPU state\n\n", key);
+
+    send_SGI_self(GIC_SGI_DUMP_HOST_STATE);
+
+    dsb(); /* Wait for SGI write to occur, or else it might be delayed
+            * until later, meaning we don't make a point of having an
+            * IPI interrupting an interrupt. */
+}
+
+static struct keyhandler dump_pcpus_keyhandler = {
+    .irq_callback = 1,
+    .diagnostic = 1,
+    .u.irq_fn = dump_pcpus,
+    .desc = "dump pcpus"
+};
+
 static DECLARE_TASKLET(dump_dom0_tasklet, NULL, 0);
 
 static void dump_dom0_action(unsigned long arg)
@@ -539,6 +558,7 @@ void __init initialize_keytable(void)
     }
     register_keyhandler('A', &toggle_alt_keyhandler);
     register_keyhandler('d', &dump_registers_keyhandler);
+    register_keyhandler('P', &dump_pcpus_keyhandler);
     register_keyhandler('h', &show_handlers_keyhandler);
     register_keyhandler('q', &dump_domains_keyhandler);
     register_keyhandler('r', &dump_runq_keyhandler);
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 87f4298..9c6f9bb 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -183,6 +183,7 @@ enum gic_sgi {
     GIC_SGI_EVENT_CHECK = 0,
     GIC_SGI_DUMP_STATE  = 1,
     GIC_SGI_CALL_FUNCTION = 2,
+    GIC_SGI_DUMP_HOST_STATE  = 3,
 };
 extern void send_SGI_mask(const cpumask_t *cpumask, enum gic_sgi sgi);
 extern void send_SGI_one(unsigned int cpu, enum gic_sgi sgi);
-- 
1.7.10.4

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Stefano Stabellini]

On Tue, 28 Jan 2014, Ian Campbell wrote:
> 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>

It looks simple enough.
Oleksandr, I would appreciate if could test the patch and tell us if it
is working well for you.


> 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
> 

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Oleksandr Tyshchenko]

[-- Attachment #1.1: Type: text/plain, Size: 7296 bytes --]

Hi, all.
Sorry for late response.
A lot of thanks to all for your advices and full solutions.
I will check the current patch and earlier solution (with tasklet_schedule)
too.


On Tue, Jan 28, 2014 at 7:50 PM, Stefano Stabellini <
stefano.stabellini@eu.citrix.com> wrote:

> On Tue, 28 Jan 2014, Ian Campbell wrote:
> > 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>
>
> It looks simple enough.
> Oleksandr, I would appreciate if could test the patch and tell us if it
> is working well for you.
>
>
> > 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
> >
>



-- 

Name | Title
GlobalLogic
P +x.xxx.xxx.xxxx  M +x.xxx.xxx.xxxx  S skype
www.globallogic.com
<http://www.globallogic.com/>
http://www.globallogic.com/email_disclaimer.txt

[-- Attachment #1.2: Type: text/html, Size: 10698 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Ian Campbell]

Ping?

I think this patch was shown to be an improvement but it lacks an actual
ack.

On Tue, 2014-01-28 at 16:51 +0000, Ian Campbell wrote:
> 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>
>  

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Julien Grall]

Hi Ian,

On 01/28/2014 04:51 PM, Ian Campbell wrote:
>      /* 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;

I'm wondering if it's necessary to set the priority for PPIs. It will be
overridden later when the interrupt will be setup. Until that time, the
interrupt is not enabled.

Regards,

-- 
Julien Grall

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Ian Campbell]

On Thu, 2014-03-13 at 12:33 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 01/28/2014 04:51 PM, Ian Campbell wrote:
> >      /* 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;
> 
> I'm wondering if it's necessary to set the priority for PPIs. It will be
> overridden later when the interrupt will be setup. Until that time, the
> interrupt is not enabled.

Strictly speaking probably not, but there isn't much harm in setting a
sane default I suppose.

Ian.

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Julien Grall]

On 03/13/2014 12:45 PM, Ian Campbell wrote:
> On Thu, 2014-03-13 at 12:33 +0000, Julien Grall wrote:
>> Hi Ian,
>>
>> On 01/28/2014 04:51 PM, Ian Campbell wrote:
>>>      /* 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;
>>
>> I'm wondering if it's necessary to set the priority for PPIs. It will be
>> overridden later when the interrupt will be setup. Until that time, the
>> interrupt is not enabled.
> 
> Strictly speaking probably not, but there isn't much harm in setting a
> sane default I suppose.

Right, I didn't see that we also do that for SPIs.

I have a minor change to request, can you divide the comment "Set PPI
and SGI priorities" in 2 to reflect was does each loop?

Except that:

Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Oleksandr Tyshchenko]

Hello, all.

I tested this patch (with debug patch) when I was looking for solution
to fix "simultaneous cross-interrupts" (see 1.1):
http://lists.xenproject.org/archives/html/xen-devel/2014-01/msg02685.html
This patch didn't fix my issue.
When the solution was founded I forgot about this patch)

Now I am using next solution:
http://marc.info/?l=xen-devel&m=139153089412362&w=2

But anyway, your patch works and looks good and I don't mind it.
Thank you.

Oleksandr Tyshchenko | Embedded Developer
GlobalLogic
www.globallogic.com

On Thu, Mar 13, 2014 at 2:56 PM, Julien Grall <julien.grall@linaro.org> wrote:
> On 03/13/2014 12:45 PM, Ian Campbell wrote:
>> On Thu, 2014-03-13 at 12:33 +0000, Julien Grall wrote:
>>> Hi Ian,
>>>
>>> On 01/28/2014 04:51 PM, Ian Campbell wrote:
>>>>      /* 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;
>>>
>>> I'm wondering if it's necessary to set the priority for PPIs. It will be
>>> overridden later when the interrupt will be setup. Until that time, the
>>> interrupt is not enabled.
>>
>> Strictly speaking probably not, but there isn't much harm in setting a
>> sane default I suppose.
>
> Right, I didn't see that we also do that for SPIs.
>
> I have a minor change to request, can you divide the comment "Set PPI
> and SGI priorities" in 2 to reflect was does each loop?
>
> Except that:
>
> Acked-by: Julien Grall <julien.grall@linaro.org>
>
> Regards,
>
> --
> Julien Grall



-- 

Name | Title
GlobalLogic
P +x.xxx.xxx.xxxx  M +x.xxx.xxx.xxxx  S skype
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Stefano Stabellini]

On Thu, 13 Mar 2014, Oleksandr Tyshchenko wrote:
> Hello, all.
> 
> I tested this patch (with debug patch) when I was looking for solution
> to fix "simultaneous cross-interrupts" (see 1.1):
> http://lists.xenproject.org/archives/html/xen-devel/2014-01/msg02685.html
> This patch didn't fix my issue.
> When the solution was founded I forgot about this patch)
> 
> Now I am using next solution:
> http://marc.info/?l=xen-devel&m=139153089412362&w=2

Speaking of which, it should probably be applied to xen-unstable now
that the gate is open?

> But anyway, your patch works and looks good and I don't mind it.
> Thank you.
> 
> Oleksandr Tyshchenko | Embedded Developer
> GlobalLogic
> www.globallogic.com
> 
> On Thu, Mar 13, 2014 at 2:56 PM, Julien Grall <julien.grall@linaro.org> wrote:
> > On 03/13/2014 12:45 PM, Ian Campbell wrote:
> >> On Thu, 2014-03-13 at 12:33 +0000, Julien Grall wrote:
> >>> Hi Ian,
> >>>
> >>> On 01/28/2014 04:51 PM, Ian Campbell wrote:
> >>>>      /* 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;
> >>>
> >>> I'm wondering if it's necessary to set the priority for PPIs. It will be
> >>> overridden later when the interrupt will be setup. Until that time, the
> >>> interrupt is not enabled.
> >>
> >> Strictly speaking probably not, but there isn't much harm in setting a
> >> sane default I suppose.
> >
> > Right, I didn't see that we also do that for SPIs.
> >
> > I have a minor change to request, can you divide the comment "Set PPI
> > and SGI priorities" in 2 to reflect was does each loop?
> >
> > Except that:
> >
> > Acked-by: Julien Grall <julien.grall@linaro.org>
> >
> > Regards,
> >
> > --
> > Julien Grall
> 
> 
> 
> -- 
> 
> Name | Title
> GlobalLogic
> P +x.xxx.xxx.xxxx  M +x.xxx.xxx.xxxx  S skype
> www.globallogic.com
> 
> http://www.globallogic.com/email_disclaimer.txt
> 

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Ian Campbell]

On Sun, 2014-03-16 at 16:38 +0000, Stefano Stabellini wrote:
> On Thu, 13 Mar 2014, Oleksandr Tyshchenko wrote:
> > Hello, all.
> > 
> > I tested this patch (with debug patch) when I was looking for solution
> > to fix "simultaneous cross-interrupts" (see 1.1):
> > http://lists.xenproject.org/archives/html/xen-devel/2014-01/msg02685.html
> > This patch didn't fix my issue.
> > When the solution was founded I forgot about this patch)
> > 
> > Now I am using next solution:
> > http://marc.info/?l=xen-devel&m=139153089412362&w=2
> 
> Speaking of which, it should probably be applied to xen-unstable now
> that the gate is open?

Julien acked it but had a couple of minor comments which I wanted to
address today. I hope to repost later on.

Ian.

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Ian Campbell]

On Mon, 2014-03-17 at 10:02 +0000, Ian Campbell wrote:
> On Sun, 2014-03-16 at 16:38 +0000, Stefano Stabellini wrote:
> > On Thu, 13 Mar 2014, Oleksandr Tyshchenko wrote:
> > > Hello, all.
> > > 
> > > I tested this patch (with debug patch) when I was looking for solution
> > > to fix "simultaneous cross-interrupts" (see 1.1):
> > > http://lists.xenproject.org/archives/html/xen-devel/2014-01/msg02685.html
> > > This patch didn't fix my issue.
> > > When the solution was founded I forgot about this patch)
> > > 
> > > Now I am using next solution:
> > > http://marc.info/?l=xen-devel&m=139153089412362&w=2
> > 
> > Speaking of which, it should probably be applied to xen-unstable now
> > that the gate is open?
> 
> Julien acked it but had a couple of minor comments which I wanted to
> address today. I hope to repost later on.

Oh you meant the route irqs to dom0 patch in the link not the one in
this thread. I'll pick that one up today as well I hope.

Ian.

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Julien Grall]

Hi Ian,

On 03/17/2014 11:29 AM, Ian Campbell wrote:
> On Mon, 2014-03-17 at 10:02 +0000, Ian Campbell wrote:
>> On Sun, 2014-03-16 at 16:38 +0000, Stefano Stabellini wrote:
>>> On Thu, 13 Mar 2014, Oleksandr Tyshchenko wrote:
>>>> Hello, all.
>>>>
>>>> I tested this patch (with debug patch) when I was looking for solution
>>>> to fix "simultaneous cross-interrupts" (see 1.1):
>>>> http://lists.xenproject.org/archives/html/xen-devel/2014-01/msg02685.html
>>>> This patch didn't fix my issue.
>>>> When the solution was founded I forgot about this patch)
>>>>
>>>> Now I am using next solution:
>>>> http://marc.info/?l=xen-devel&m=139153089412362&w=2
>>>
>>> Speaking of which, it should probably be applied to xen-unstable now
>>> that the gate is open?
>>
>> Julien acked it but had a couple of minor comments which I wanted to
>> address today. I hope to repost later on.
> 
> Oh you meant the route irqs to dom0 patch in the link not the one in
> this thread. I'll pick that one up today as well I hope.

With the IPI patch, I don't think we need the route irqs to CPU0 patch
(at least for Xen 4.5). It would be better to backport it for Xen 4.4.

Regards,

-- 
Julien Grall

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Ian Campbell]

On Mon, 2014-03-17 at 11:34 +0000, Julien Grall wrote:
> Hi Ian,
> 
> On 03/17/2014 11:29 AM, Ian Campbell wrote:
> > On Mon, 2014-03-17 at 10:02 +0000, Ian Campbell wrote:
> >> On Sun, 2014-03-16 at 16:38 +0000, Stefano Stabellini wrote:
> >>> On Thu, 13 Mar 2014, Oleksandr Tyshchenko wrote:
> >>>> Hello, all.
> >>>>
> >>>> I tested this patch (with debug patch) when I was looking for solution
> >>>> to fix "simultaneous cross-interrupts" (see 1.1):
> >>>> http://lists.xenproject.org/archives/html/xen-devel/2014-01/msg02685.html
> >>>> This patch didn't fix my issue.
> >>>> When the solution was founded I forgot about this patch)
> >>>>
> >>>> Now I am using next solution:
> >>>> http://marc.info/?l=xen-devel&m=139153089412362&w=2
> >>>
> >>> Speaking of which, it should probably be applied to xen-unstable now
> >>> that the gate is open?
> >>
> >> Julien acked it but had a couple of minor comments which I wanted to
> >> address today. I hope to repost later on.
> > 
> > Oh you meant the route irqs to dom0 patch in the link not the one in
> > this thread. I'll pick that one up today as well I hope.
> 
> With the IPI patch, I don't think we need the route irqs to CPU0 patch
> (at least for Xen 4.5). It would be better to backport it for Xen 4.4.

Oleksndr said above that the IPI patch didn't help but the CPU0 one did.
But rereading I wonder if I am misinterpreting what was said.

Ian.

Re: [PATCH] xen: arm: increase priority of SGIs used as IPIs

[Ian Campbell]

On Mon, 2014-03-17 at 11:36 +0000, Ian Campbell wrote:
> On Mon, 2014-03-17 at 11:34 +0000, Julien Grall wrote:
> > Hi Ian,
> > 
> > On 03/17/2014 11:29 AM, Ian Campbell wrote:
> > > On Mon, 2014-03-17 at 10:02 +0000, Ian Campbell wrote:
> > >> On Sun, 2014-03-16 at 16:38 +0000, Stefano Stabellini wrote:
> > >>> On Thu, 13 Mar 2014, Oleksandr Tyshchenko wrote:
> > >>>> Hello, all.
> > >>>>
> > >>>> I tested this patch (with debug patch) when I was looking for solution
> > >>>> to fix "simultaneous cross-interrupts" (see 1.1):
> > >>>> http://lists.xenproject.org/archives/html/xen-devel/2014-01/msg02685.html
> > >>>> This patch didn't fix my issue.
> > >>>> When the solution was founded I forgot about this patch)
> > >>>>
> > >>>> Now I am using next solution:
> > >>>> http://marc.info/?l=xen-devel&m=139153089412362&w=2
> > >>>
> > >>> Speaking of which, it should probably be applied to xen-unstable now
> > >>> that the gate is open?
> > >>
> > >> Julien acked it but had a couple of minor comments which I wanted to
> > >> address today. I hope to repost later on.
> > > 
> > > Oh you meant the route irqs to dom0 patch in the link not the one in
> > > this thread. I'll pick that one up today as well I hope.
> > 
> > With the IPI patch, I don't think we need the route irqs to CPU0 patch
> > (at least for Xen 4.5). It would be better to backport it for Xen 4.4.
> 
> Oleksndr said above that the IPI patch didn't help but the CPU0 one did.
> But rereading I wonder if I am misinterpreting what was said.

Because of this uncertainty I'm going to wait for confirmation of what
is going on before I apply "xen/arm: route irqs to cpu0".

Ian.