All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/18] xen/arm: Interrupt management reworking
@ 2014-04-22 12:58 Julien Grall
  2014-04-22 12:58 ` [PATCH v4 01/18] xen/arm: timer: replace timer_dt_irq by timer_get_irq Julien Grall
                   ` (19 more replies)
  0 siblings, 20 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

Hello,

This is the fourth version of this serie to rework interrupt management for
ARM.

Major changes in v4:
    - Use a custom single linked list for handling multiple action
    - Slightly rework patch #9 (was acked by Ian)
    - Remove dependency on Stefano's series for Patch #10
    - Change platform_get_irq error code to -1 (impact patch #14 acked by Ian)

For all changes see in each patch.

This series is a dependency for the ARM SMMU drivers.

A working tree can be found here:
    git://xenbits.xen.org/people/julieng/xen-unstable.git branch interrupt-mgmt-v4

Sincerely yours,

Julien Grall (18):
  xen/arm: timer: replace timer_dt_irq by timer_get_irq
  xen/arm: IRQ: Use default irq callback from common code for
    no_irq_type
  xen/arm: IRQ: Rename irq_cfg into arch_irq_desc
  xen/arm: IRQ: move gic {,un}lock in gic_set_irq_properties
  xen/arm: IRQ: drop irq parameter in __setup_irq
  xen/arm: IRQ: remove __init from setup_dt_irq, request_dt_irq and
    release_irq
  xen/arm: IRQ: Rework gic_route_irq_to_guest function
  xen/arm: IRQ: Move IRQ management from gic.c to irq.c
  xen/arm: IRQ Introduce irq_get_domain
  xen/arm: IRQ: Require desc.lock be held by callers of
    hw_irq_controller callbacks
  xen/arm: IRQ: Defer routing IRQ to Xen until setup_irq() call
  xen/arm: IRQ: Do not allow IRQ to be shared between domains and XEN
  xen/serial: remove serial_dt_irq
  xen/arm: IRQ: Store IRQ type in arch_irq_desc
  xen/arm: IRQ: Replace {request,setup}_dt_irq by {request,setup}_irq
  xen: IRQ: Add dev_id parameter to release_irq
  xen/arm: IRQ: extend {request,setup}_irq to take an irqflags in
    parameter
  xen/arm: IRQ: Handle multiple action per IRQ

 xen/arch/arm/domain_build.c              |   32 +--
 xen/arch/arm/gic.c                       |  230 +++++----------------
 xen/arch/arm/irq.c                       |  321 ++++++++++++++++++++++++++++--
 xen/arch/arm/platforms/xgene-storm.c     |    2 +-
 xen/arch/arm/setup.c                     |    5 +-
 xen/arch/arm/smpboot.c                   |    2 -
 xen/arch/arm/time.c                      |   43 ++--
 xen/arch/arm/vgic.c                      |   10 +
 xen/arch/arm/vtimer.c                    |    4 +-
 xen/arch/x86/hpet.c                      |    2 +-
 xen/arch/x86/i8259.c                     |    2 +-
 xen/arch/x86/irq.c                       |   11 +-
 xen/arch/x86/time.c                      |    2 +-
 xen/drivers/char/exynos4210-uart.c       |   23 +--
 xen/drivers/char/ns16550.c               |   31 +--
 xen/drivers/char/omap-uart.c             |   23 +--
 xen/drivers/char/pl011.c                 |   24 +--
 xen/drivers/char/serial.c                |    9 -
 xen/drivers/passthrough/amd/iommu_init.c |    2 +-
 xen/drivers/passthrough/vtd/iommu.c      |    2 +-
 xen/include/asm-arm/config.h             |    2 +
 xen/include/asm-arm/gic.h                |   16 +-
 xen/include/asm-arm/irq.h                |   12 +-
 xen/include/asm-arm/time.h               |    7 +-
 xen/include/xen/irq.h                    |   11 +-
 xen/include/xen/serial.h                 |    5 -
 26 files changed, 471 insertions(+), 362 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 01/18] xen/arm: timer: replace timer_dt_irq by timer_get_irq
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 02/18] xen/arm: IRQ: Use default irq callback from common code for no_irq_type Julien Grall
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

The function is nearly only used to retrieve the IRQ number.

There is one place where the IRQ type is used (in domain_build.c) but
as the timer IRQ is virtualised for guest we might not have the same property
(e.g active-low level sensitive interrupt).

Replace timer_dt_irq by timer_get_irq which will return the IRQ number.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v2:
        - Patch added
---
 xen/arch/arm/domain_build.c |   23 +++++++++++++----------
 xen/arch/arm/time.c         |    4 ++--
 xen/arch/arm/vtimer.c       |    4 ++--
 xen/include/asm-arm/time.h  |    4 ++--
 4 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 187e071..7bc9bf6 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -616,7 +616,7 @@ static int make_timer_node(const struct domain *d, void *fdt,
     u32 len;
     const void *compatible;
     int res;
-    const struct dt_irq *irq;
+    unsigned int irq;
     gic_interrupt_t intrs[3];
     u32 clock_frequency;
     bool_t clock_valid;
@@ -645,17 +645,20 @@ static int make_timer_node(const struct domain *d, void *fdt,
     if ( res )
         return res;
 
-    irq = timer_dt_irq(TIMER_PHYS_SECURE_PPI);
-    DPRINT("  Secure interrupt %u\n", irq->irq);
-    set_interrupt_ppi(intrs[0], irq->irq, 0xf, irq->type);
+    /* The timer IRQ is emulated by Xen. It always exposes an active-low
+     * level-sensitive interrupt */
 
-    irq = timer_dt_irq(TIMER_PHYS_NONSECURE_PPI);
-    DPRINT("  Non secure interrupt %u\n", irq->irq);
-    set_interrupt_ppi(intrs[1], irq->irq, 0xf, irq->type);
+    irq = timer_get_irq(TIMER_PHYS_SECURE_PPI);
+    DPRINT("  Secure interrupt %u\n", irq);
+    set_interrupt_ppi(intrs[0], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
-    irq = timer_dt_irq(TIMER_VIRT_PPI);
-    DPRINT("  Virt interrupt %u\n", irq->irq);
-    set_interrupt_ppi(intrs[2], irq->irq, 0xf, irq->type);
+    irq = timer_get_irq(TIMER_PHYS_NONSECURE_PPI);
+    DPRINT("  Non secure interrupt %u\n", irq);
+    set_interrupt_ppi(intrs[1], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
+
+    irq = timer_get_irq(TIMER_VIRT_PPI);
+    DPRINT("  Virt interrupt %u\n", irq);
+    set_interrupt_ppi(intrs[2], irq, 0xf, DT_IRQ_TYPE_LEVEL_LOW);
 
     res = fdt_property_interrupts(fdt, intrs, 3);
     if ( res )
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 8dd4bea..7cad888 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -50,11 +50,11 @@ unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
 
 static struct dt_irq timer_irq[MAX_TIMER_PPI];
 
-const struct dt_irq *timer_dt_irq(enum timer_ppi ppi)
+unsigned int timer_get_irq(enum timer_ppi ppi)
 {
     ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
 
-    return &timer_irq[ppi];
+    return timer_irq[ppi].irq;
 }
 
 /*static inline*/ s_time_t ticks_to_ns(uint64_t ticks)
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index cb690bb..4822d4f 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -65,7 +65,7 @@ int vcpu_vtimer_init(struct vcpu *v)
     t->ctl = 0;
     t->cval = NOW();
     t->irq = d0
-        ? timer_dt_irq(TIMER_PHYS_NONSECURE_PPI)->irq
+        ? timer_get_irq(TIMER_PHYS_NONSECURE_PPI)
         : GUEST_TIMER_PHYS_NS_PPI;
     t->v = v;
 
@@ -73,7 +73,7 @@ int vcpu_vtimer_init(struct vcpu *v)
     init_timer(&t->timer, virt_timer_expired, t, v->processor);
     t->ctl = 0;
     t->irq = d0
-        ? timer_dt_irq(TIMER_VIRT_PPI)->irq
+        ? timer_get_irq(TIMER_VIRT_PPI)
         : GUEST_TIMER_VIRT_PPI;
     t->v = v;
 
diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
index d10c737..9bbab0b 100644
--- a/xen/include/asm-arm/time.h
+++ b/xen/include/asm-arm/time.h
@@ -22,8 +22,8 @@ enum timer_ppi
     MAX_TIMER_PPI = 4,
 };
 
-/* Get one of the timer IRQ description */
-const struct dt_irq* timer_dt_irq(enum timer_ppi ppi);
+/* Get one of the timer IRQ number */
+unsigned int timer_get_irq(enum timer_ppi ppi);
 
 /* Route timer's IRQ on this CPU */
 extern void __cpuinit route_timer_interrupt(void);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 02/18] xen/arm: IRQ: Use default irq callback from common code for no_irq_type
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
  2014-04-22 12:58 ` [PATCH v4 01/18] xen/arm: timer: replace timer_dt_irq by timer_get_irq Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 03/18] xen/arm: IRQ: Rename irq_cfg into arch_irq_desc Julien Grall
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

Most of no_irq_type callback are already defined in common/irq.c. We don't
need to recreate our own callbacks.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v2:
        - Patch added
---
 xen/arch/arm/irq.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 3e326b0..5d1ed7f 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -27,23 +27,19 @@
 
 #include <asm/gic.h>
 
-static void enable_none(struct irq_desc *irq) { }
-static unsigned int startup_none(struct irq_desc *irq) { return 0; }
-static void disable_none(struct irq_desc *irq) { }
 static void ack_none(struct irq_desc *irq)
 {
     printk("unexpected IRQ trap at irq %02x\n", irq->irq);
 }
 
-#define shutdown_none   disable_none
-#define end_none        enable_none
+static void end_none(struct irq_desc *irq) { }
 
 hw_irq_controller no_irq_type = {
     .typename = "none",
-    .startup = startup_none,
-    .shutdown = shutdown_none,
-    .enable = enable_none,
-    .disable = disable_none,
+    .startup = irq_startup_none,
+    .shutdown = irq_shutdown_none,
+    .enable = irq_enable_none,
+    .disable = irq_disable_none,
     .ack = ack_none,
     .end = end_none
 };
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 03/18] xen/arm: IRQ: Rename irq_cfg into arch_irq_desc
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
  2014-04-22 12:58 ` [PATCH v4 01/18] xen/arm: timer: replace timer_dt_irq by timer_get_irq Julien Grall
  2014-04-22 12:58 ` [PATCH v4 02/18] xen/arm: IRQ: Use default irq callback from common code for no_irq_type Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 04/18] xen/arm: IRQ: move gic {, un}lock in gic_set_irq_properties Julien Grall
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

irq_cfg is never used in the code and arch_irq_desc is an alias to irq_cfg.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v2:
        - Patch added
---
 xen/include/asm-arm/irq.h |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 7c20703..3197aec 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -14,8 +14,7 @@ struct arch_pirq
 {
 };
 
-struct irq_cfg {
-#define arch_irq_desc irq_cfg
+struct arch_irq_desc {
     int eoi_cpu;
 };
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 04/18] xen/arm: IRQ: move gic {, un}lock in gic_set_irq_properties
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (2 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 03/18] xen/arm: IRQ: Rename irq_cfg into arch_irq_desc Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 05/18] xen/arm: IRQ: drop irq parameter in __setup_irq Julien Grall
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

The function gic_set_irq_properties is only called in two places:
    - gic_route_irq: the gic.lock is only taken for the call to the
    former function.
    - gic_route_irq_to_guest: the gic.lock is taken for the duration of
    the function. But the lock is only useful when gic_set_irq_properties.

So we can safely move the lock in gic_set_irq_properties and restrict the
critical section for the gic.lock in gic_route_irq_to_guest.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v2:
        - Remove useless comment about gic.lock
---
 xen/arch/arm/gic.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 8168b7b..842231a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -221,7 +221,6 @@ static hw_irq_controller gic_guest_irq_type = {
 };
 
 /*
- * - needs to be called with gic.lock held
  * - needs to be called with a valid cpu_mask, ie each cpu in the mask has
  * already called gic_cpu_init
  */
@@ -231,7 +230,11 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level,
 {
     volatile unsigned char *bytereg;
     uint32_t cfg, edgebit;
-    unsigned int mask = gic_cpu_mask(cpu_mask);
+    unsigned int mask;
+
+    spin_lock(&gic.lock);
+
+    mask = gic_cpu_mask(cpu_mask);
 
     /* Set edge / level */
     cfg = GICD[GICD_ICFGR + irq / 16];
@@ -250,6 +253,7 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level,
     bytereg = (unsigned char *) (GICD + GICD_IPRIORITYR);
     bytereg[irq] = priority;
 
+    spin_unlock(&gic.lock);
 }
 
 /* Program the GIC to route an interrupt */
@@ -272,9 +276,7 @@ static int gic_route_irq(unsigned int irq, bool_t level,
 
     desc->handler = &gic_host_irq_type;
 
-    spin_lock(&gic.lock);
     gic_set_irq_properties(irq, level, cpu_mask, priority);
-    spin_unlock(&gic.lock);
 
     spin_unlock_irqrestore(&desc->lock, flags);
     return 0;
@@ -779,7 +781,6 @@ int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq,
     action->free_on_release = 1;
 
     spin_lock_irqsave(&desc->lock, flags);
-    spin_lock(&gic.lock);
 
     desc->handler = &gic_guest_irq_type;
     desc->status |= IRQ_GUEST;
@@ -800,7 +801,6 @@ int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq,
     p->desc = desc;
 
 out:
-    spin_unlock(&gic.lock);
     spin_unlock_irqrestore(&desc->lock, flags);
     return retval;
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 05/18] xen/arm: IRQ: drop irq parameter in __setup_irq
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (3 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 04/18] xen/arm: IRQ: move gic {, un}lock in gic_set_irq_properties Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 06/18] xen/arm: IRQ: remove __init from setup_dt_irq, request_dt_irq and release_irq Julien Grall
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

The IRQ number is already provided by desc and __setup_irq doesn't use
it in any case.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v2:
        - Patch added
---
 xen/arch/arm/gic.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 842231a..1934adf 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -594,8 +594,7 @@ void __init release_irq(unsigned int irq)
         xfree(action);
 }
 
-static int __setup_irq(struct irq_desc *desc, unsigned int irq,
-                       struct irqaction *new)
+static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
 {
     if ( desc->action != NULL )
         return -EBUSY;
@@ -615,7 +614,7 @@ int __init setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
     desc = irq_to_desc(irq->irq);
 
     spin_lock_irqsave(&desc->lock, flags);
-    rc = __setup_irq(desc, irq->irq, new);
+    rc = __setup_irq(desc, new);
     spin_unlock_irqrestore(&desc->lock, flags);
 
     if ( !rc )
@@ -790,7 +789,7 @@ int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq,
     gic_set_irq_properties(irq->irq, level, cpumask_of(smp_processor_id()),
                            GIC_PRI_IRQ);
 
-    retval = __setup_irq(desc, irq->irq, action);
+    retval = __setup_irq(desc, action);
     if (retval) {
         xfree(action);
         goto out;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 06/18] xen/arm: IRQ: remove __init from setup_dt_irq, request_dt_irq and release_irq
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (4 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 05/18] xen/arm: IRQ: drop irq parameter in __setup_irq Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 07/18] xen/arm: IRQ: Rework gic_route_irq_to_guest function Julien Grall
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

These functions will be used in SMMU driver which request interrupt when
a device is assigned to a guest.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v2:
        - Patch added
---
 xen/arch/arm/gic.c        |    4 ++--
 xen/arch/arm/irq.c        |    6 +++---
 xen/include/asm-arm/irq.h |    8 ++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 1934adf..8d3c155 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -570,7 +570,7 @@ void gic_route_spis(void)
     }
 }
 
-void __init release_irq(unsigned int irq)
+void release_irq(unsigned int irq)
 {
     struct irq_desc *desc;
     unsigned long flags;
@@ -605,7 +605,7 @@ static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
     return 0;
 }
 
-int __init setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
+int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
 {
     int rc;
     unsigned long flags;
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 5d1ed7f..b3bfebc 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -89,9 +89,9 @@ void __cpuinit init_secondary_IRQ(void)
     BUG_ON(init_local_irq_data() < 0);
 }
 
-int __init request_dt_irq(const struct dt_irq *irq,
-        void (*handler)(int, void *, struct cpu_user_regs *),
-        const char *devname, void *dev_id)
+int request_dt_irq(const struct dt_irq *irq,
+                   void (*handler)(int, void *, struct cpu_user_regs *),
+                   const char *devname, void *dev_id)
 {
     struct irqaction *action;
     int retval;
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 3197aec..9380987 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -39,10 +39,10 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq);
 void init_IRQ(void);
 void init_secondary_IRQ(void);
 
-int __init request_dt_irq(const struct dt_irq *irq,
-                          void (*handler)(int, void *, struct cpu_user_regs *),
-                          const char *devname, void *dev_id);
-int __init setup_dt_irq(const struct dt_irq *irq, struct irqaction *new);
+int request_dt_irq(const struct dt_irq *irq,
+                   void (*handler)(int, void *, struct cpu_user_regs *),
+                   const char *devname, void *dev_id);
+int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new);
 
 #endif /* _ASM_HW_IRQ_H */
 /*
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 07/18] xen/arm: IRQ: Rework gic_route_irq_to_guest function
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (5 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 06/18] xen/arm: IRQ: remove __init from setup_dt_irq, request_dt_irq and release_irq Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 08/18] xen/arm: IRQ: Move IRQ management from gic.c to irq.c Julien Grall
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

The function gic_route_irq_to_guest contains code which is not related to the
GIC. Split the function in 2 parts:

- route_dt_irq_to_guest: setup the desc
- gic_route_irq_to_guest: setup correctly the GIC and the desc handler

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v3:
        - Patch added (split from "Move IRQ management from gic.c to irq.c")
---
 xen/arch/arm/domain_build.c          |    2 +-
 xen/arch/arm/gic.c                   |   43 ++++++++++++++++++++++------------
 xen/arch/arm/platforms/xgene-storm.c |    2 +-
 xen/include/asm-arm/gic.h            |    3 ---
 xen/include/asm-arm/irq.h            |    3 +++
 5 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 7bc9bf6..5b636c8 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -726,7 +726,7 @@ static int map_device(struct domain *d, const struct dt_device_node *dev)
 
         DPRINT("irq %u = %u type = 0x%x\n", i, irq.irq, irq.type);
         /* Don't check return because the IRQ can be use by multiple device */
-        gic_route_irq_to_guest(d, &irq, dt_node_name(dev));
+        route_dt_irq_to_guest(d, &irq, dt_node_name(dev));
     }
 
     /* Map the address ranges */
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 8d3c155..90b129d 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -282,6 +282,27 @@ static int gic_route_irq(unsigned int irq, bool_t level,
     return 0;
 }
 
+/* Program the GIC to route an interrupt to a guest
+ *   - desc.lock must be held
+ */
+static void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
+                                   bool_t level, const cpumask_t *cpu_mask,
+                                   unsigned int priority)
+{
+    struct pending_irq *p;
+    ASSERT(spin_is_locked(&desc->lock));
+
+    desc->handler = &gic_guest_irq_type;
+    desc->status |= IRQ_GUEST;
+
+    gic_set_irq_properties(desc->irq, level, cpumask_of(smp_processor_id()),
+                           GIC_PRI_IRQ);
+
+    /* TODO: do not assume delivery to vcpu0 */
+    p = irq_to_pending(d->vcpu[0], desc->irq);
+    p->desc = desc;
+}
+
 /* Program the GIC to route an interrupt with a dt_irq */
 void gic_route_dt_irq(const struct dt_irq *irq, const cpumask_t *cpu_mask,
                       unsigned int priority)
@@ -761,15 +782,14 @@ void gic_inject(void)
         gic_inject_irq_start();
 }
 
-int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq,
-                           const char * devname)
+int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
+                          const char * devname)
 {
     struct irqaction *action;
     struct irq_desc *desc = irq_to_desc(irq->irq);
     unsigned long flags;
     int retval;
     bool_t level;
-    struct pending_irq *p;
 
     action = xmalloc(struct irqaction);
     if (!action)
@@ -781,23 +801,16 @@ int gic_route_irq_to_guest(struct domain *d, const struct dt_irq *irq,
 
     spin_lock_irqsave(&desc->lock, flags);
 
-    desc->handler = &gic_guest_irq_type;
-    desc->status |= IRQ_GUEST;
-
-    level = dt_irq_is_level_triggered(irq);
-
-    gic_set_irq_properties(irq->irq, level, cpumask_of(smp_processor_id()),
-                           GIC_PRI_IRQ);
-
     retval = __setup_irq(desc, action);
-    if (retval) {
+    if ( retval )
+    {
         xfree(action);
         goto out;
     }
 
-    /* TODO: do not assume delivery to vcpu0 */
-    p = irq_to_pending(d->vcpu[0], irq->irq);
-    p->desc = desc;
+    level = dt_irq_is_level_triggered(irq);
+    gic_route_irq_to_guest(d, desc, level, cpumask_of(smp_processor_id()),
+                           GIC_PRI_IRQ);
 
 out:
     spin_unlock_irqrestore(&desc->lock, flags);
diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c
index af3b71c..70aab73 100644
--- a/xen/arch/arm/platforms/xgene-storm.c
+++ b/xen/arch/arm/platforms/xgene-storm.c
@@ -66,7 +66,7 @@ static int map_one_spi(struct domain *d, const char *what,
 
     printk("Additional IRQ %u (%s)\n", irq.irq, what);
 
-    ret = gic_route_irq_to_guest(d, &irq, what);
+    ret = route_dt_irq_to_guest(d, &irq, what);
     if ( ret )
         printk("Failed to route %s to dom%d\n", what, d->domain_id);
 
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 071280b..9489c04 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -181,9 +181,6 @@ extern void __cpuinit init_maintenance_interrupt(void);
 extern void gic_set_guest_irq(struct vcpu *v, unsigned int irq,
         unsigned int state, unsigned int priority);
 extern void gic_remove_from_queues(struct vcpu *v, unsigned int virtual_irq);
-extern int gic_route_irq_to_guest(struct domain *d,
-                                  const struct dt_irq *irq,
-                                  const char * devname);
 
 /* Accept an interrupt from the GIC and dispatch its handler */
 extern void gic_interrupt(struct cpu_user_regs *regs, int is_fiq);
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 9380987..b52c26f 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -44,6 +44,9 @@ int request_dt_irq(const struct dt_irq *irq,
                    const char *devname, void *dev_id);
 int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new);
 
+int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
+                          const char *devname);
+
 #endif /* _ASM_HW_IRQ_H */
 /*
  * Local variables:
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 08/18] xen/arm: IRQ: Move IRQ management from gic.c to irq.c
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (6 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 07/18] xen/arm: IRQ: Rework gic_route_irq_to_guest function Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 09/18] xen/arm: IRQ Introduce irq_get_domain Julien Grall
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

The file gic.c contains functions and variables which is not related to the GIC:
    - release_irq
    - setup_irq
    - gic_route_irq_to_guest
    - {,local_}irq_desc

Move all theses functions/variables in irq.c

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v3:
        - Split the patch in 2: refactoring + code motion
    Changes in v2:
        - Patch added
---
 xen/arch/arm/gic.c        |  102 ++-------------------------------------------
 xen/arch/arm/irq.c        |   97 ++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/gic.h |    4 ++
 3 files changed, 104 insertions(+), 99 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 90b129d..77dfecf 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -52,8 +52,6 @@ static struct {
     spinlock_t lock;
 } gic;
 
-static irq_desc_t irq_desc[NR_IRQS];
-static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc);
 static DEFINE_PER_CPU(uint64_t, lr_mask);
 
 static unsigned nr_lrs;
@@ -88,12 +86,6 @@ unsigned int gic_number_lines(void)
     return gic.lines;
 }
 
-irq_desc_t *__irq_to_desc(int irq)
-{
-    if (irq < NR_LOCAL_IRQS) return &this_cpu(local_irq_desc)[irq];
-    return &irq_desc[irq-NR_LOCAL_IRQS];
-}
-
 void gic_save_state(struct vcpu *v)
 {
     int i;
@@ -285,9 +277,9 @@ static int gic_route_irq(unsigned int irq, bool_t level,
 /* Program the GIC to route an interrupt to a guest
  *   - desc.lock must be held
  */
-static void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
-                                   bool_t level, const cpumask_t *cpu_mask,
-                                   unsigned int priority)
+void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
+                            bool_t level, const cpumask_t *cpu_mask,
+                            unsigned int priority)
 {
     struct pending_irq *p;
     ASSERT(spin_is_locked(&desc->lock));
@@ -591,59 +583,6 @@ void gic_route_spis(void)
     }
 }
 
-void release_irq(unsigned int irq)
-{
-    struct irq_desc *desc;
-    unsigned long flags;
-   struct irqaction *action;
-
-    desc = irq_to_desc(irq);
-
-    desc->handler->shutdown(desc);
-
-    spin_lock_irqsave(&desc->lock,flags);
-    action = desc->action;
-    desc->action  = NULL;
-    desc->status &= ~IRQ_GUEST;
-
-    spin_unlock_irqrestore(&desc->lock,flags);
-
-    /* Wait to make sure it's not being used on another CPU */
-    do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
-
-    if (action && action->free_on_release)
-        xfree(action);
-}
-
-static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
-{
-    if ( desc->action != NULL )
-        return -EBUSY;
-
-    desc->action  = new;
-    dsb(sy);
-
-    return 0;
-}
-
-int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
-{
-    int rc;
-    unsigned long flags;
-    struct irq_desc *desc;
-
-    desc = irq_to_desc(irq->irq);
-
-    spin_lock_irqsave(&desc->lock, flags);
-    rc = __setup_irq(desc, new);
-    spin_unlock_irqrestore(&desc->lock, flags);
-
-    if ( !rc )
-        desc->handler->startup(desc);
-
-    return rc;
-}
-
 static inline void gic_set_lr(int lr, struct pending_irq *p,
         unsigned int state)
 {
@@ -782,41 +721,6 @@ void gic_inject(void)
         gic_inject_irq_start();
 }
 
-int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
-                          const char * devname)
-{
-    struct irqaction *action;
-    struct irq_desc *desc = irq_to_desc(irq->irq);
-    unsigned long flags;
-    int retval;
-    bool_t level;
-
-    action = xmalloc(struct irqaction);
-    if (!action)
-        return -ENOMEM;
-
-    action->dev_id = d;
-    action->name = devname;
-    action->free_on_release = 1;
-
-    spin_lock_irqsave(&desc->lock, flags);
-
-    retval = __setup_irq(desc, action);
-    if ( retval )
-    {
-        xfree(action);
-        goto out;
-    }
-
-    level = dt_irq_is_level_triggered(irq);
-    gic_route_irq_to_guest(d, desc, level, cpumask_of(smp_processor_id()),
-                           GIC_PRI_IRQ);
-
-out:
-    spin_unlock_irqrestore(&desc->lock, flags);
-    return retval;
-}
-
 static void do_sgi(struct cpu_user_regs *regs, int othercpu, enum gic_sgi sgi)
 {
     /* Lower the priority */
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index b3bfebc..f3a30bd 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -44,6 +44,15 @@ hw_irq_controller no_irq_type = {
     .end = end_none
 };
 
+static irq_desc_t irq_desc[NR_IRQS];
+static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc);
+
+irq_desc_t *__irq_to_desc(int irq)
+{
+    if (irq < NR_LOCAL_IRQS) return &this_cpu(local_irq_desc)[irq];
+    return &irq_desc[irq-NR_LOCAL_IRQS];
+}
+
 int __init arch_init_one_irq_desc(struct irq_desc *desc)
 {
     return 0;
@@ -188,6 +197,94 @@ out_no_end:
     irq_exit();
 }
 
+void release_irq(unsigned int irq)
+{
+    struct irq_desc *desc;
+    unsigned long flags;
+   struct irqaction *action;
+
+    desc = irq_to_desc(irq);
+
+    desc->handler->shutdown(desc);
+
+    spin_lock_irqsave(&desc->lock,flags);
+    action = desc->action;
+    desc->action  = NULL;
+    desc->status &= ~IRQ_GUEST;
+
+    spin_unlock_irqrestore(&desc->lock,flags);
+
+    /* Wait to make sure it's not being used on another CPU */
+    do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
+
+    if ( action && action->free_on_release )
+        xfree(action);
+}
+
+static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
+{
+    if ( desc->action != NULL )
+        return -EBUSY;
+
+    desc->action  = new;
+    dsb(sy);
+
+    return 0;
+}
+
+int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
+{
+    int rc;
+    unsigned long flags;
+    struct irq_desc *desc;
+
+    desc = irq_to_desc(irq->irq);
+
+    spin_lock_irqsave(&desc->lock, flags);
+    rc = __setup_irq(desc, new);
+    spin_unlock_irqrestore(&desc->lock, flags);
+
+    if ( !rc )
+        desc->handler->startup(desc);
+
+    return rc;
+}
+
+int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
+                          const char * devname)
+{
+    struct irqaction *action;
+    struct irq_desc *desc = irq_to_desc(irq->irq);
+    unsigned long flags;
+    int retval;
+    bool_t level;
+
+    action = xmalloc(struct irqaction);
+    if (!action)
+        return -ENOMEM;
+
+    action->dev_id = d;
+    action->name = devname;
+    action->free_on_release = 1;
+
+    spin_lock_irqsave(&desc->lock, flags);
+
+    retval = __setup_irq(desc, action);
+    if ( retval )
+    {
+        xfree(action);
+        goto out;
+    }
+
+    level = dt_irq_is_level_triggered(irq);
+    gic_route_irq_to_guest(d, desc, level, cpumask_of(smp_processor_id()),
+                           GIC_PRI_IRQ);
+
+out:
+    spin_unlock_irqrestore(&desc->lock, flags);
+    return retval;
+}
+
 /*
  * pirq event channels. We don't use these on ARM, instead we use the
  * features of the GIC to inject virtualised normal interrupts.
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 9489c04..0e6e325 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -153,6 +153,7 @@
 
 #ifndef __ASSEMBLY__
 #include <xen/device_tree.h>
+#include <xen/irq.h>
 
 #define DT_MATCH_GIC    DT_MATCH_COMPATIBLE("arm,cortex-a15-gic"), \
                         DT_MATCH_COMPATIBLE("arm,cortex-a7-gic")
@@ -170,6 +171,9 @@ extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
 extern void gic_route_dt_irq(const struct dt_irq *irq,
                              const cpumask_t *cpu_mask,
                              unsigned int priority);
+extern void gic_route_irq_to_guest(struct domain *, struct irq_desc *desc,
+                                   bool_t level, const cpumask_t *cpu_mask,
+                                   unsigned int priority);
 extern void gic_route_ppis(void);
 extern void gic_route_spis(void);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 09/18] xen/arm: IRQ Introduce irq_get_domain
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (7 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 08/18] xen/arm: IRQ: Move IRQ management from gic.c to irq.c Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 10/18] xen/arm: IRQ: Require desc.lock be held by callers of hw_irq_controller callbacks Julien Grall
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

This function retrieves a domain from an IRQ. It will be used in several
places (such as do_IRQ) to avoid duplicated code when multiple action will be
supported.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v4:
        - Return dom_xen when the IRQ is not routed to a guest

    Changes in v2:
        - Patch added
---
 xen/arch/arm/irq.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index f3a30bd..26574ca 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -98,6 +98,18 @@ void __cpuinit init_secondary_IRQ(void)
     BUG_ON(init_local_irq_data() < 0);
 }
 
+static inline struct domain *irq_get_domain(struct irq_desc *desc)
+{
+    ASSERT(spin_is_locked(&desc->lock));
+
+    if ( !(desc->status & IRQ_GUEST) )
+        return dom_xen;
+
+    ASSERT(desc->action != NULL);
+
+    return desc->action->dev_id;
+}
+
 int request_dt_irq(const struct dt_irq *irq,
                    void (*handler)(int, void *, struct cpu_user_regs *),
                    const char *devname, void *dev_id)
@@ -156,7 +168,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
 
     if ( desc->status & IRQ_GUEST )
     {
-        struct domain *d = action->dev_id;
+        struct domain *d = irq_get_domain(desc);
 
         desc->handler->end(desc);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 10/18] xen/arm: IRQ: Require desc.lock be held by callers of hw_irq_controller callbacks
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (8 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 09/18] xen/arm: IRQ Introduce irq_get_domain Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 11/18] xen/arm: IRQ: Defer routing IRQ to Xen until setup_irq() call Julien Grall
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

When multiple action are supported, gic_irq_{startup,shutdown} will have
to be called in the same critical section as setup/release.
Otherwise there is a race condition if at the same time CPU A is calling
release_dt_irq and CPU B is calling setup_dt_irq.

This could end up with the IRQ not being enabled.

At the same time, modify gic_irq_{enable,disable} to require desc.lock be held.

With both of theses changes, ARM's locking requirements is the same as x86's.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v4:
        - Fix typo in commit message
        - Don't depend on Stefano's interrupt series (will likely be
          pushed before)

    Changes in v3:
        - Update commit message
        - Require desc.lock also for gic_irq_{enable,disable}
        - irqflags is unsigned long not unsigned int

    Changes in v2:
        - Fix typoes in commit message
        - Move this patch earlier in the series => move shutdown() in
        release_irq and gic_route_irq
---
 xen/arch/arm/gic.c  |   21 +++++++++++----------
 xen/arch/arm/irq.c  |    6 ++++--
 xen/arch/arm/vgic.c |   10 ++++++++++
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 77dfecf..29ecb49 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -128,14 +128,14 @@ static void gic_irq_enable(struct irq_desc *desc)
     int irq = desc->irq;
     unsigned long flags;
 
-    spin_lock_irqsave(&desc->lock, flags);
-    spin_lock(&gic.lock);
+    ASSERT(spin_is_locked(&desc->lock));
+
+    spin_lock_irqsave(&gic.lock, flags);
     desc->status &= ~IRQ_DISABLED;
     dsb(sy);
     /* Enable routing */
     GICD[GICD_ISENABLER + irq / 32] = (1u << (irq % 32));
-    spin_unlock(&gic.lock);
-    spin_unlock_irqrestore(&desc->lock, flags);
+    spin_unlock_irqrestore(&gic.lock, flags);
 }
 
 static void gic_irq_disable(struct irq_desc *desc)
@@ -143,18 +143,19 @@ static void gic_irq_disable(struct irq_desc *desc)
     int irq = desc->irq;
     unsigned long flags;
 
-    spin_lock_irqsave(&desc->lock, flags);
-    spin_lock(&gic.lock);
+    ASSERT(spin_is_locked(&desc->lock));
+
+    spin_lock_irqsave(&gic.lock, flags);
     /* Disable routing */
     GICD[GICD_ICENABLER + irq / 32] = (1u << (irq % 32));
     desc->status |= IRQ_DISABLED;
-    spin_unlock(&gic.lock);
-    spin_unlock_irqrestore(&desc->lock, flags);
+    spin_unlock_irqrestore(&gic.lock, flags);
 }
 
 static unsigned int gic_irq_startup(struct irq_desc *desc)
 {
     gic_irq_enable(desc);
+
     return 0;
 }
 
@@ -261,11 +262,11 @@ static int gic_route_irq(unsigned int irq, bool_t level,
     if ( desc->action != NULL )
         return -EBUSY;
 
+    spin_lock_irqsave(&desc->lock, flags);
+
     /* Disable interrupt */
     desc->handler->shutdown(desc);
 
-    spin_lock_irqsave(&desc->lock, flags);
-
     desc->handler = &gic_host_irq_type;
 
     gic_set_irq_properties(irq, level, cpu_mask, priority);
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 26574ca..b6dd9de 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -217,9 +217,10 @@ void release_irq(unsigned int irq)
 
     desc = irq_to_desc(irq);
 
+    spin_lock_irqsave(&desc->lock,flags);
+
     desc->handler->shutdown(desc);
 
-    spin_lock_irqsave(&desc->lock,flags);
     action = desc->action;
     desc->action  = NULL;
     desc->status &= ~IRQ_GUEST;
@@ -254,11 +255,12 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
 
     spin_lock_irqsave(&desc->lock, flags);
     rc = __setup_irq(desc, new);
-    spin_unlock_irqrestore(&desc->lock, flags);
 
     if ( !rc )
         desc->handler->startup(desc);
 
+    spin_unlock_irqrestore(&desc->lock, flags);
+
     return rc;
 }
 
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 4a7f8c0..1b95003 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -374,6 +374,7 @@ static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
     const unsigned long mask = r;
     struct pending_irq *p;
     unsigned int irq;
+    unsigned long flags;
     int i = 0;
 
     while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
@@ -382,7 +383,11 @@ static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
         clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
         gic_remove_from_queues(v, irq);
         if ( p->desc != NULL )
+        {
+            spin_lock_irqsave(&p->desc->lock, flags);
             p->desc->handler->disable(p->desc);
+            spin_unlock_irqrestore(&p->desc->lock, flags);
+        }
         i++;
     }
 }
@@ -392,6 +397,7 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
     const unsigned long mask = r;
     struct pending_irq *p;
     unsigned int irq;
+    unsigned long flags;
     int i = 0;
 
     while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
@@ -401,7 +407,11 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
         if ( !list_empty(&p->inflight) && !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
             gic_set_guest_irq(v, irq, GICH_LR_PENDING, p->priority);
         if ( p->desc != NULL )
+        {
+            spin_lock_irqsave(&p->desc->lock, flags);
             p->desc->handler->enable(p->desc);
+            spin_unlock_irqrestore(&p->desc->lock, flags);
+        }
         i++;
     }
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 11/18] xen/arm: IRQ: Defer routing IRQ to Xen until setup_irq() call
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (9 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 10/18] xen/arm: IRQ: Require desc.lock be held by callers of hw_irq_controller callbacks Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 12/18] xen/arm: IRQ: Do not allow IRQ to be shared between domains and XEN Julien Grall
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

When an IRQ is handling by Xen, setup is done in 2 steps:
    - Route the IRQ to the current CPU and set priorities
    - Set up the handler

For PPIs, these steps are called on every cpu. For SPIs, they are only called
on the boot CPU.

Dividing the setup in two step complicates the code when a new driver is
added to Xen (for instance a SMMU driver). Xen can safely route the IRQ
when the driver sets up the interrupt handler.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v3:
        - Don't need to initialiase disabled
        - Typoes and update commit message

    Changes in v2:
        - Fix typo in commit message
        - s/SGI/SPI/ in comments
        - Rename gic_route_dt_irq into gic_route_irq_to_xen which is
        taking a desc now
        - Call setup_irq before initializing the GIC IRQ as the first one
        can fail.
---
 xen/arch/arm/gic.c         |   63 +++++++-------------------------------------
 xen/arch/arm/irq.c         |   24 ++++++++++++++++-
 xen/arch/arm/setup.c       |    2 --
 xen/arch/arm/smpboot.c     |    2 --
 xen/arch/arm/time.c        |   11 --------
 xen/include/asm-arm/gic.h  |   10 +++----
 xen/include/asm-arm/time.h |    3 ---
 7 files changed, 36 insertions(+), 79 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 29ecb49..577d85b 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -249,30 +249,20 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level,
     spin_unlock(&gic.lock);
 }
 
-/* Program the GIC to route an interrupt */
-static int gic_route_irq(unsigned int irq, bool_t level,
-                         const cpumask_t *cpu_mask, unsigned int priority)
+/* Program the GIC to route an interrupt to the host (i.e. Xen)
+ * - needs to be called with desc.lock held
+ */
+void gic_route_irq_to_xen(struct irq_desc *desc, bool_t level,
+                          const cpumask_t *cpu_mask, unsigned int priority)
 {
-    struct irq_desc *desc = irq_to_desc(irq);
-    unsigned long flags;
-
     ASSERT(priority <= 0xff);     /* Only 8 bits of priority */
-    ASSERT(irq < gic.lines);      /* Can't route interrupts that don't exist */
-
-    if ( desc->action != NULL )
-        return -EBUSY;
-
-    spin_lock_irqsave(&desc->lock, flags);
-
-    /* Disable interrupt */
-    desc->handler->shutdown(desc);
+    ASSERT(desc->irq < gic.lines);/* Can't route interrupts that don't exist */
+    ASSERT(desc->status & IRQ_DISABLED);
+    ASSERT(spin_is_locked(&desc->lock));
 
     desc->handler = &gic_host_irq_type;
 
-    gic_set_irq_properties(irq, level, cpu_mask, priority);
-
-    spin_unlock_irqrestore(&desc->lock, flags);
-    return 0;
+    gic_set_irq_properties(desc->irq, level, cpu_mask, priority);
 }
 
 /* Program the GIC to route an interrupt to a guest
@@ -296,17 +286,6 @@ void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
     p->desc = desc;
 }
 
-/* Program the GIC to route an interrupt with a dt_irq */
-void gic_route_dt_irq(const struct dt_irq *irq, const cpumask_t *cpu_mask,
-                      unsigned int priority)
-{
-    bool_t level;
-
-    level = dt_irq_is_level_triggered(irq);
-
-    gic_route_irq(irq->irq, level, cpu_mask, priority);
-}
-
 static void __init gic_dist_init(void)
 {
     uint32_t type;
@@ -560,30 +539,6 @@ void gic_disable_cpu(void)
     spin_unlock(&gic.lock);
 }
 
-void gic_route_ppis(void)
-{
-    /* GIC maintenance */
-    gic_route_dt_irq(&gic.maintenance, cpumask_of(smp_processor_id()),
-                     GIC_PRI_IRQ);
-    /* Route timer interrupt */
-    route_timer_interrupt();
-}
-
-void gic_route_spis(void)
-{
-    int seridx;
-    const struct dt_irq *irq;
-
-    for ( seridx = 0; seridx <= SERHND_IDX; seridx++ )
-    {
-        if ( (irq = serial_dt_irq(seridx)) == NULL )
-            continue;
-
-        gic_route_dt_irq(irq, cpumask_of(smp_processor_id()),
-                         GIC_PRI_IRQ);
-    }
-}
-
 static inline void gic_set_lr(int lr, struct pending_irq *p,
         unsigned int state)
 {
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index b6dd9de..9f1ca40 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -250,15 +250,37 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
     int rc;
     unsigned long flags;
     struct irq_desc *desc;
+    bool_t disabled;
 
     desc = irq_to_desc(irq->irq);
 
     spin_lock_irqsave(&desc->lock, flags);
+
+    disabled = (desc->action == NULL);
+
     rc = __setup_irq(desc, new);
+    if ( rc )
+        goto err;
 
-    if ( !rc )
+    /* First time the IRQ is setup */
+    if ( disabled )
+    {
+        bool_t level;
+
+        level = dt_irq_is_level_triggered(irq);
+        /* It's fine to use smp_processor_id() because:
+         * For PPI: irq_desc is banked
+         * For SPI: we don't care for now which CPU will receive the
+         * interrupt
+         * TODO: Handle case where SPI is setup on different CPU than
+         * the targeted CPU and the priority.
+         */
+        gic_route_irq_to_xen(desc, level, cpumask_of(smp_processor_id()),
+                             GIC_PRI_IRQ);
         desc->handler->startup(desc);
+    }
 
+err:
     spin_unlock_irqrestore(&desc->lock, flags);
 
     return rc;
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 6b77a4c..dec5950 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -719,8 +719,6 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     init_IRQ();
 
-    gic_route_ppis();
-    gic_route_spis();
     xsm_dt_init();
 
     init_maintenance_interrupt();
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 7f28b68..cf149da 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -287,8 +287,6 @@ void __cpuinit start_secondary(unsigned long boot_phys_offset,
 
     init_secondary_IRQ();
 
-    gic_route_ppis();
-
     init_maintenance_interrupt();
     init_timer_interrupt();
 
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 7cad888..ce96337 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -218,17 +218,6 @@ static void vtimer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
     vgic_vcpu_inject_irq(current, current->arch.virt_timer.irq, 1);
 }
 
-/* Route timer's IRQ on this CPU */
-void __cpuinit route_timer_interrupt(void)
-{
-    gic_route_dt_irq(&timer_irq[TIMER_PHYS_NONSECURE_PPI],
-                     cpumask_of(smp_processor_id()), GIC_PRI_IRQ);
-    gic_route_dt_irq(&timer_irq[TIMER_HYP_PPI],
-                     cpumask_of(smp_processor_id()), GIC_PRI_IRQ);
-    gic_route_dt_irq(&timer_irq[TIMER_VIRT_PPI],
-                     cpumask_of(smp_processor_id()), GIC_PRI_IRQ);
-}
-
 /* Set up the timer interrupt on this CPU */
 void __cpuinit init_timer_interrupt(void)
 {
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 0e6e325..b750b17 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -167,15 +167,13 @@ extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,int virtual);
 extern void vgic_clear_pending_irqs(struct vcpu *v);
 extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
 
-/* Program the GIC to route an interrupt with a dt_irq */
-extern void gic_route_dt_irq(const struct dt_irq *irq,
-                             const cpumask_t *cpu_mask,
-                             unsigned int priority);
+/* Program the GIC to route an interrupt */
+extern void gic_route_irq_to_xen(struct irq_desc *desc, bool_t level,
+                                 const cpumask_t *cpu_mask,
+                                 unsigned int priority);
 extern void gic_route_irq_to_guest(struct domain *, struct irq_desc *desc,
                                    bool_t level, const cpumask_t *cpu_mask,
                                    unsigned int priority);
-extern void gic_route_ppis(void);
-extern void gic_route_spis(void);
 
 extern void gic_inject(void);
 extern void gic_clear_pending_irqs(struct vcpu *v);
diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
index 9bbab0b..d544b5b 100644
--- a/xen/include/asm-arm/time.h
+++ b/xen/include/asm-arm/time.h
@@ -25,9 +25,6 @@ enum timer_ppi
 /* Get one of the timer IRQ number */
 unsigned int timer_get_irq(enum timer_ppi ppi);
 
-/* Route timer's IRQ on this CPU */
-extern void __cpuinit route_timer_interrupt(void);
-
 /* Set up the timer interrupt on this CPU */
 extern void __cpuinit init_timer_interrupt(void);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 12/18] xen/arm: IRQ: Do not allow IRQ to be shared between domains and XEN
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (10 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 11/18] xen/arm: IRQ: Defer routing IRQ to Xen until setup_irq() call Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 13/18] xen/serial: remove serial_dt_irq Julien Grall
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

The current dt_route_irq_to_guest implementation sets IRQ_GUEST even if the
IRQ is correctly setup.

An IRQ can be shared between devices, if the devices are not assigned to the
same domain or Xen, then this could result in routing the IRQ to the domain
instead of Xen ...

Also avoid to relying on wrong the behaviour when Xen is routing an IRQ to
DOM0. Therefore check the return code from route_dt_irq_to_guest in
map_device.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v4:
        - Don't forget to free action if sanity check has failed

    Changes in v3:
        - Fix typoes and update commit message

    Changes in v2:
        - Use EBUSY instead of EADDRINUSE for error code
        - Don't hardcode the domain in error message
        - Fix typo in error message
        - Use irq_get_domain
---
 xen/arch/arm/domain_build.c |    9 +++++++--
 xen/arch/arm/irq.c          |   41 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 5b636c8..9dcff1c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -725,8 +725,13 @@ static int map_device(struct domain *d, const struct dt_device_node *dev)
         }
 
         DPRINT("irq %u = %u type = 0x%x\n", i, irq.irq, irq.type);
-        /* Don't check return because the IRQ can be use by multiple device */
-        route_dt_irq_to_guest(d, &irq, dt_node_name(dev));
+        res = route_dt_irq_to_guest(d, &irq, dt_node_name(dev));
+        if ( res )
+        {
+            printk(XENLOG_ERR "Unable to route IRQ %u to domain %u\n",
+                   irq.irq, d->domain_id);
+            return res;
+        }
     }
 
     /* Map the address ranges */
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 9f1ca40..44696e7 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -256,6 +256,16 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
 
     spin_lock_irqsave(&desc->lock, flags);
 
+    if ( desc->status & IRQ_GUEST )
+    {
+        struct domain *d = irq_get_domain(desc);
+
+        spin_unlock_irqrestore(&desc->lock, flags);
+        printk(XENLOG_ERR "ERROR: IRQ %u is already in use by the domain %u\n",
+               irq->irq, d->domain_id);
+        return -EBUSY;
+    }
+
     disabled = (desc->action == NULL);
 
     rc = __setup_irq(desc, new);
@@ -292,7 +302,7 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
     struct irqaction *action;
     struct irq_desc *desc = irq_to_desc(irq->irq);
     unsigned long flags;
-    int retval;
+    int retval = 0;
     bool_t level;
 
     action = xmalloc(struct irqaction);
@@ -305,19 +315,42 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
 
     spin_lock_irqsave(&desc->lock, flags);
 
-    retval = __setup_irq(desc, action);
-    if ( retval )
+    /* If the IRQ is already used by someone
+     *  - If it's the same domain -> Xen doesn't need to update the IRQ desc
+     *  - Otherwise -> For now, don't allow the IRQ to be shared between
+     *  Xen and domains.
+     */
+    if ( desc->action != NULL )
     {
-        xfree(action);
+        struct domain *ad = irq_get_domain(desc);
+
+        if ( (desc->status & IRQ_GUEST) && d == ad )
+            goto out;
+
+        if ( desc->status & IRQ_GUEST )
+            printk(XENLOG_ERR "ERROR: IRQ %u is already used by domain %u\n",
+                   irq->irq, ad->domain_id);
+        else
+            printk(XENLOG_ERR "ERROR: IRQ %u is already used by Xen\n",
+                   irq->irq);
+        retval = -EBUSY;
         goto out;
     }
 
+    retval = __setup_irq(desc, action);
+    if ( retval )
+        goto out;
+
     level = dt_irq_is_level_triggered(irq);
     gic_route_irq_to_guest(d, desc, level, cpumask_of(smp_processor_id()),
                            GIC_PRI_IRQ);
+    spin_unlock_irqrestore(&desc->lock, flags);
+    return 0;
 
 out:
     spin_unlock_irqrestore(&desc->lock, flags);
+    xfree(action);
+
     return retval;
 }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 13/18] xen/serial: remove serial_dt_irq
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (11 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 12/18] xen/arm: IRQ: Do not allow IRQ to be shared between domains and XEN Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc Julien Grall
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel
  Cc: stefano.stabellini, Keir Fraser, Julien Grall, tim, ian.campbell

This function was only used for ARM IRQ routing which has been removed in an
earlier patch.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
CC: Keir Fraser <keir@xen.org>

---
    Changes in v2:
        - Specify in the commit this patch is ARM specific
---
 xen/drivers/char/exynos4210-uart.c |    8 --------
 xen/drivers/char/ns16550.c         |   11 -----------
 xen/drivers/char/omap-uart.c       |    8 --------
 xen/drivers/char/pl011.c           |    8 --------
 xen/drivers/char/serial.c          |    9 ---------
 xen/include/xen/serial.h           |    5 -----
 6 files changed, 49 deletions(-)

diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index d49e1fe..370539c 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -275,13 +275,6 @@ static int __init exynos4210_uart_irq(struct serial_port *port)
     return uart->irq.irq;
 }
 
-static const struct dt_irq __init *exynos4210_uart_dt_irq(struct serial_port *port)
-{
-    struct exynos4210_uart *uart = port->uart;
-
-    return &uart->irq;
-}
-
 static const struct vuart_info *exynos4210_vuart_info(struct serial_port *port)
 {
     struct exynos4210_uart *uart = port->uart;
@@ -299,7 +292,6 @@ static struct uart_driver __read_mostly exynos4210_uart_driver = {
     .putc         = exynos4210_uart_putc,
     .getc         = exynos4210_uart_getc,
     .irq          = exynos4210_uart_irq,
-    .dt_irq_get   = exynos4210_uart_dt_irq,
     .vuart_info   = exynos4210_vuart_info,
 };
 
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 4aa33d5..21f086a 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -742,14 +742,6 @@ static int __init ns16550_irq(struct serial_port *port)
     return ((uart->irq > 0) ? uart->irq : -1);
 }
 
-#ifdef HAS_DEVICE_TREE
-static const struct dt_irq __init *ns16550_dt_irq(struct serial_port *port)
-{
-    struct ns16550 *uart = port->uart;
-    return &uart->dt_irq;
-}
-#endif
-
 #ifdef CONFIG_ARM
 static const struct vuart_info *ns16550_vuart_info(struct serial_port *port)
 {
@@ -769,9 +761,6 @@ static struct uart_driver __read_mostly ns16550_driver = {
     .putc         = ns16550_putc,
     .getc         = ns16550_getc,
     .irq          = ns16550_irq,
-#ifdef HAS_DEVICE_TREE
-    .dt_irq_get   = ns16550_dt_irq,
-#endif
 #ifdef CONFIG_ARM
     .vuart_info   = ns16550_vuart_info,
 #endif
diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index 49ae1a4..b8da509 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -262,13 +262,6 @@ static int __init omap_uart_irq(struct serial_port *port)
     return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
 }
 
-static const struct dt_irq __init *omap_uart_dt_irq(struct serial_port *port)
-{
-    struct omap_uart *uart = port->uart;
-
-    return &uart->irq;
-}
-
 static const struct vuart_info *omap_vuart_info(struct serial_port *port)
 {
     struct omap_uart *uart = port->uart;
@@ -286,7 +279,6 @@ static struct uart_driver __read_mostly omap_uart_driver = {
     .putc = omap_uart_putc,
     .getc = omap_uart_getc,
     .irq = omap_uart_irq,
-    .dt_irq_get = omap_uart_dt_irq,
     .vuart_info = omap_vuart_info,
 };
 
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 90bf0c6..459e686 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -189,13 +189,6 @@ static int __init pl011_irq(struct serial_port *port)
     return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
 }
 
-static const struct dt_irq __init *pl011_dt_irq(struct serial_port *port)
-{
-    struct pl011 *uart = port->uart;
-
-    return &uart->irq;
-}
-
 static const struct vuart_info *pl011_vuart(struct serial_port *port)
 {
     struct pl011 *uart = port->uart;
@@ -213,7 +206,6 @@ static struct uart_driver __read_mostly pl011_driver = {
     .putc         = pl011_putc,
     .getc         = pl011_getc,
     .irq          = pl011_irq,
-    .dt_irq_get   = pl011_dt_irq,
     .vuart_info   = pl011_vuart,
 };
 
diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
index 9b006f2..44026b1 100644
--- a/xen/drivers/char/serial.c
+++ b/xen/drivers/char/serial.c
@@ -500,15 +500,6 @@ int __init serial_irq(int idx)
     return -1;
 }
 
-const struct dt_irq __init *serial_dt_irq(int idx)
-{
-    if ( (idx >= 0) && (idx < ARRAY_SIZE(com)) &&
-         com[idx].driver && com[idx].driver->dt_irq_get )
-        return com[idx].driver->dt_irq_get(&com[idx]);
-
-    return NULL;
-}
-
 const struct vuart_info *serial_vuart_info(int idx)
 {
     if ( (idx >= 0) && (idx < ARRAY_SIZE(com)) &&
diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
index f38c9b7..9f4451b 100644
--- a/xen/include/xen/serial.h
+++ b/xen/include/xen/serial.h
@@ -81,8 +81,6 @@ struct uart_driver {
     int  (*getc)(struct serial_port *, char *);
     /* Get IRQ number for this port's serial line: returns -1 if none. */
     int  (*irq)(struct serial_port *);
-    /* Get IRQ device node for this port's serial line: returns NULL if none. */
-    const struct dt_irq *(*dt_irq_get)(struct serial_port *);
     /* Get serial information */
     const struct vuart_info *(*vuart_info)(struct serial_port *);
 };
@@ -135,9 +133,6 @@ void serial_end_log_everything(int handle);
 /* Return irq number for specified serial port (identified by index). */
 int serial_irq(int idx);
 
-/* Return irq device node for specified serial port (identified by index). */
-const struct dt_irq *serial_dt_irq(int idx);
-
 /* Retrieve basic UART information to emulate it (base address, size...) */
 const struct vuart_info* serial_vuart_info(int idx);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (12 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 13/18] xen/serial: remove serial_dt_irq Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-23 12:26   ` Julien Grall
  2014-04-23 14:04   ` Ian Campbell
  2014-04-22 12:58 ` [PATCH v4 15/18] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq Julien Grall
                   ` (5 subsequent siblings)
  19 siblings, 2 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell

For now, ARM uses different IRQ functions to setup an interrupt handler. This
is a bit annoying for common driver because we have to add idefery when
an IRQ is setup (see ns16550_init_postirq for an example).

To avoid to completely fork the IRQ management code, we can introduce a field
to store the IRQ type (e.g level/edge ...).

This patch also adds platform_get_irq which will retrieve the IRQ from the
device tree and setup correctly the IRQ type.

In order to use this solution, we have to move init_IRQ earlier for the boot
CPU. It's fine because the code only depends on percpu.

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

---
    Changes in v4:
        - Add an ASSERT to check if irq_set_type hasn't failed for PPI
        on other CPU than 0
        - platform_get_irq return -1 in case of error.

    Changes in v3:
        - irqflags is unsigned long not unsigned int
        - fix comment
        - don't need to set IRQ type when NONE is used
        (already set at startup).

    Changes in v2:
        - Patch added
---
 xen/arch/arm/gic.c        |   23 ++++++------
 xen/arch/arm/irq.c        |   85 +++++++++++++++++++++++++++++++++++++++++----
 xen/arch/arm/setup.c      |    3 +-
 xen/include/asm-arm/gic.h |    5 ++-
 xen/include/asm-arm/irq.h |    3 ++
 5 files changed, 98 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 577d85b..4a6ed35 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -216,14 +216,19 @@ static hw_irq_controller gic_guest_irq_type = {
 /*
  * - needs to be called with a valid cpu_mask, ie each cpu in the mask has
  * already called gic_cpu_init
+ * - desc.lock must be held
  */
-static void gic_set_irq_properties(unsigned int irq, bool_t level,
+static void gic_set_irq_properties(struct irq_desc *desc,
                                    const cpumask_t *cpu_mask,
                                    unsigned int priority)
 {
     volatile unsigned char *bytereg;
     uint32_t cfg, edgebit;
     unsigned int mask;
+    unsigned int irq = desc->irq;
+    unsigned int type = desc->arch.type;
+
+    ASSERT(spin_is_locked(&desc->lock));
 
     spin_lock(&gic.lock);
 
@@ -232,9 +237,9 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level,
     /* Set edge / level */
     cfg = GICD[GICD_ICFGR + irq / 16];
     edgebit = 2u << (2 * (irq % 16));
-    if ( level )
+    if ( type & DT_IRQ_TYPE_LEVEL_MASK )
         cfg &= ~edgebit;
-    else
+    else if ( type & DT_IRQ_TYPE_EDGE_BOTH )
         cfg |= edgebit;
     GICD[GICD_ICFGR + irq / 16] = cfg;
 
@@ -252,8 +257,8 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level,
 /* Program the GIC to route an interrupt to the host (i.e. Xen)
  * - needs to be called with desc.lock held
  */
-void gic_route_irq_to_xen(struct irq_desc *desc, bool_t level,
-                          const cpumask_t *cpu_mask, unsigned int priority)
+void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
+                          unsigned int priority)
 {
     ASSERT(priority <= 0xff);     /* Only 8 bits of priority */
     ASSERT(desc->irq < gic.lines);/* Can't route interrupts that don't exist */
@@ -262,15 +267,14 @@ void gic_route_irq_to_xen(struct irq_desc *desc, bool_t level,
 
     desc->handler = &gic_host_irq_type;
 
-    gic_set_irq_properties(desc->irq, level, cpu_mask, priority);
+    gic_set_irq_properties(desc, cpu_mask, priority);
 }
 
 /* Program the GIC to route an interrupt to a guest
  *   - desc.lock must be held
  */
 void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
-                            bool_t level, const cpumask_t *cpu_mask,
-                            unsigned int priority)
+                            const cpumask_t *cpu_mask, unsigned int priority)
 {
     struct pending_irq *p;
     ASSERT(spin_is_locked(&desc->lock));
@@ -278,8 +282,7 @@ void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
     desc->handler = &gic_guest_irq_type;
     desc->status |= IRQ_GUEST;
 
-    gic_set_irq_properties(desc->irq, level, cpumask_of(smp_processor_id()),
-                           GIC_PRI_IRQ);
+    gic_set_irq_properties(desc, cpumask_of(smp_processor_id()), GIC_PRI_IRQ);
 
     /* TODO: do not assume delivery to vcpu0 */
     p = irq_to_pending(d->vcpu[0], desc->irq);
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 44696e7..f1d6398 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -55,6 +55,7 @@ irq_desc_t *__irq_to_desc(int irq)
 
 int __init arch_init_one_irq_desc(struct irq_desc *desc)
 {
+    desc->arch.type = DT_IRQ_TYPE_NONE;
     return 0;
 }
 
@@ -82,6 +83,12 @@ static int __cpuinit init_local_irq_data(void)
         init_one_irq_desc(desc);
         desc->irq = irq;
         desc->action  = NULL;
+
+        /* PPIs are include in local_irqs, we have to copy the IRQ type from
+         * CPU0 otherwise we may miss the type if the IRQ type has been
+         * set early.
+         */
+        desc->arch.type = per_cpu(local_irq_desc, 0)[irq].arch.type;
     }
 
     return 0;
@@ -275,9 +282,6 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
     /* First time the IRQ is setup */
     if ( disabled )
     {
-        bool_t level;
-
-        level = dt_irq_is_level_triggered(irq);
         /* It's fine to use smp_processor_id() because:
          * For PPI: irq_desc is banked
          * For SPI: we don't care for now which CPU will receive the
@@ -285,7 +289,8 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
          * TODO: Handle case where SPI is setup on different CPU than
          * the targeted CPU and the priority.
          */
-        gic_route_irq_to_xen(desc, level, cpumask_of(smp_processor_id()),
+        desc->arch.type = irq->type;
+        gic_route_irq_to_xen(desc, cpumask_of(smp_processor_id()),
                              GIC_PRI_IRQ);
         desc->handler->startup(desc);
     }
@@ -303,7 +308,6 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
     struct irq_desc *desc = irq_to_desc(irq->irq);
     unsigned long flags;
     int retval = 0;
-    bool_t level;
 
     action = xmalloc(struct irqaction);
     if (!action)
@@ -341,8 +345,8 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
     if ( retval )
         goto out;
 
-    level = dt_irq_is_level_triggered(irq);
-    gic_route_irq_to_guest(d, desc, level, cpumask_of(smp_processor_id()),
+    desc->arch.type = irq->type;
+    gic_route_irq_to_guest(d, desc, cpumask_of(smp_processor_id()),
                            GIC_PRI_IRQ);
     spin_unlock_irqrestore(&desc->lock, flags);
     return 0;
@@ -383,6 +387,73 @@ void pirq_set_affinity(struct domain *d, int pirq, const cpumask_t *mask)
     BUG();
 }
 
+static inline int irq_set_type(struct irq_desc *desc, unsigned int type)
+{
+    unsigned long flags;
+    int ret = -EBUSY;
+
+    if ( type == DT_IRQ_TYPE_NONE )
+        return 0;
+
+    spin_lock_irqsave(&desc->lock, flags);
+
+    if ( desc->arch.type != DT_IRQ_TYPE_NONE && desc->arch.type != type )
+        goto err;
+
+    desc->arch.type = type;
+
+    ret = 0;
+
+err:
+    spin_unlock_irqrestore(&desc->lock, flags);
+    return ret;
+}
+
+int platform_get_irq(const struct dt_device_node *device,
+                     int index)
+{
+    struct dt_irq dt_irq;
+    struct irq_desc *desc;
+    unsigned int type, irq;
+    int res;
+
+    res = dt_device_get_irq(device, index, &dt_irq);
+    if ( res )
+        return 0;
+
+    irq = dt_irq.irq;
+    type = dt_irq.type;
+
+    /* Setup the IRQ type */
+    if ( irq < NR_LOCAL_IRQS )
+    {
+        unsigned int cpu;
+        /* For PPIs, we need to set IRQ type on every online CPUs */
+        for_each_cpu( cpu, &cpu_online_map )
+        {
+            desc = &per_cpu(local_irq_desc, cpu)[irq];
+            res = irq_set_type(desc, type);
+            if ( res )
+            {
+                /*
+                 * For PPIs the IRQ type is the same on every CPU. It
+                 * should not fail to other CPU than CPU0
+                 */
+                ASSERT(cpu == 0);
+                return -1;
+            }
+        }
+    }
+    else
+    {
+        res = irq_set_type(irq_to_desc(irq), type);
+        if ( res )
+            return -1;
+    }
+
+    return irq;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index dec5950..40ba26e 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -688,6 +688,8 @@ void __init start_xen(unsigned long boot_phys_offset,
     dt_unflatten_host_device_tree();
     dt_irq_xlate = gic_irq_xlate;
 
+    init_IRQ();
+
     dt_uart_init();
     console_init_preirq();
 
@@ -717,7 +719,6 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     tasklet_subsys_init();
 
-    init_IRQ();
 
     xsm_dt_init();
 
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index b750b17..80f8dd2 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -168,11 +168,10 @@ extern void vgic_clear_pending_irqs(struct vcpu *v);
 extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
 
 /* Program the GIC to route an interrupt */
-extern void gic_route_irq_to_xen(struct irq_desc *desc, bool_t level,
-                                 const cpumask_t *cpu_mask,
+extern void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
                                  unsigned int priority);
 extern void gic_route_irq_to_guest(struct domain *, struct irq_desc *desc,
-                                   bool_t level, const cpumask_t *cpu_mask,
+                                   const cpumask_t *cpu_mask,
                                    unsigned int priority);
 
 extern void gic_inject(void);
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index b52c26f..93d2128 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -16,6 +16,7 @@ struct arch_pirq
 
 struct arch_irq_desc {
     int eoi_cpu;
+    unsigned int type;
 };
 
 #define NR_LOCAL_IRQS	32
@@ -46,6 +47,8 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new);
 
 int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
                           const char *devname);
+int platform_get_irq(const struct dt_device_node *device,
+                     int index);
 
 #endif /* _ASM_HW_IRQ_H */
 /*
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 15/18] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (13 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 13:26   ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 16/18] xen: IRQ: Add dev_id parameter to release_irq Julien Grall
                   ` (4 subsequent siblings)
  19 siblings, 1 reply; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel
  Cc: stefano.stabellini, Keir Fraser, Julien Grall, tim, ian.campbell

Now that irq_desc stores the type of the IRQ (e.g level/edge,...), we don't
need to use specific IRQ function for ARM.

Also replace every call to dt_device_get_irq by platform_get_irq which is
a wrapper to this function and setup the IRQ type correctly.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Keir Fraser <keir@xen.org>

---
    Changes in v4:
        - platform_get_irq now returns -1 in case of error.

    Changes in v3:
        - Fix typoes in commit message

    Changes in v2:
        - Patch added
---
 xen/arch/arm/gic.c                 |   13 +++++++------
 xen/arch/arm/irq.c                 |   23 +++++++++++------------
 xen/arch/arm/time.c                |   30 ++++++++++++++++--------------
 xen/drivers/char/exynos4210-uart.c |   15 ++++++++-------
 xen/drivers/char/ns16550.c         |   18 ++++--------------
 xen/drivers/char/omap-uart.c       |   15 ++++++++-------
 xen/drivers/char/pl011.c           |   18 ++++++++++--------
 xen/include/asm-arm/irq.h          |    5 -----
 8 files changed, 64 insertions(+), 73 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 4a6ed35..cb40b0a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -47,7 +47,7 @@ static struct {
     paddr_t hbase;       /* Address of virtual interface registers */
     paddr_t vbase;       /* Address of virtual cpu interface registers */
     unsigned int lines;  /* Number of interrupts (SPIs + PPIs + SGIs) */
-    struct dt_irq maintenance; /* IRQ maintenance */
+    unsigned int maintenance_irq; /* IRQ maintenance */
     unsigned int cpus;
     spinlock_t lock;
 } gic;
@@ -431,9 +431,10 @@ void __init gic_init(void)
     if ( res || !gic.vbase || (gic.vbase & ~PAGE_MASK) )
         panic("GIC: Cannot find a valid address for the virtual CPU");
 
-    res = dt_device_get_irq(node, 0, &gic.maintenance);
-    if ( res )
+    res = platform_get_irq(node, 0);
+    if ( res < 0 )
         panic("GIC: Cannot find the maintenance IRQ");
+    gic.maintenance_irq = res;
 
     /* Set the GIC as the primary interrupt controller */
     dt_interrupt_controller = node;
@@ -447,7 +448,7 @@ void __init gic_init(void)
               "        gic_vcpu_addr=%"PRIpaddr"\n"
               "        gic_maintenance_irq=%u\n",
               gic.dbase, gic.cbase, gic.hbase, gic.vbase,
-              gic.maintenance.irq);
+              gic.maintenance_irq);
 
     if ( (gic.dbase & ~PAGE_MASK) || (gic.cbase & ~PAGE_MASK) ||
          (gic.hbase & ~PAGE_MASK) || (gic.vbase & ~PAGE_MASK) )
@@ -884,8 +885,8 @@ void gic_dump_info(struct vcpu *v)
 
 void __cpuinit init_maintenance_interrupt(void)
 {
-    request_dt_irq(&gic.maintenance, maintenance_interrupt,
-                   "irq-maintenance", NULL);
+    request_irq(gic.maintenance_irq, maintenance_interrupt,
+                "irq-maintenance", NULL);
 }
 
 /*
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index f1d6398..a09ca25 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -117,9 +117,9 @@ static inline struct domain *irq_get_domain(struct irq_desc *desc)
     return desc->action->dev_id;
 }
 
-int request_dt_irq(const struct dt_irq *irq,
-                   void (*handler)(int, void *, struct cpu_user_regs *),
-                   const char *devname, void *dev_id)
+int request_irq(unsigned int irq,
+                void (*handler)(int, void *, struct cpu_user_regs *),
+                const char *devname, void *dev_id)
 {
     struct irqaction *action;
     int retval;
@@ -130,13 +130,13 @@ int request_dt_irq(const struct dt_irq *irq,
      * which interrupt is which (messes up the interrupt freeing
      * logic etc).
      */
-    if (irq->irq >= nr_irqs)
+    if ( irq >= nr_irqs )
         return -EINVAL;
-    if (!handler)
+    if ( !handler )
         return -EINVAL;
 
     action = xmalloc(struct irqaction);
-    if (!action)
+    if ( !action )
         return -ENOMEM;
 
     action->handler = handler;
@@ -144,8 +144,8 @@ int request_dt_irq(const struct dt_irq *irq,
     action->dev_id = dev_id;
     action->free_on_release = 1;
 
-    retval = setup_dt_irq(irq, action);
-    if (retval)
+    retval = setup_irq(irq, action);
+    if ( retval )
         xfree(action);
 
     return retval;
@@ -252,14 +252,14 @@ static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
     return 0;
 }
 
-int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
+int setup_irq(unsigned int irq, struct irqaction *new)
 {
     int rc;
     unsigned long flags;
     struct irq_desc *desc;
     bool_t disabled;
 
-    desc = irq_to_desc(irq->irq);
+    desc = irq_to_desc(irq);
 
     spin_lock_irqsave(&desc->lock, flags);
 
@@ -269,7 +269,7 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
 
         spin_unlock_irqrestore(&desc->lock, flags);
         printk(XENLOG_ERR "ERROR: IRQ %u is already in use by the domain %u\n",
-               irq->irq, d->domain_id);
+               irq, d->domain_id);
         return -EBUSY;
     }
 
@@ -289,7 +289,6 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
          * TODO: Handle case where SPI is setup on different CPU than
          * the targeted CPU and the priority.
          */
-        desc->arch.type = irq->type;
         gic_route_irq_to_xen(desc, cpumask_of(smp_processor_id()),
                              GIC_PRI_IRQ);
         desc->handler->startup(desc);
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index ce96337..0bff747 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -48,13 +48,13 @@ uint64_t __read_mostly boot_count;
  * register-mapped time source in the SoC. */
 unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
 
-static struct dt_irq timer_irq[MAX_TIMER_PPI];
+static unsigned int timer_irq[MAX_TIMER_PPI];
 
 unsigned int timer_get_irq(enum timer_ppi ppi)
 {
     ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
 
-    return timer_irq[ppi].irq;
+    return timer_irq[ppi];
 }
 
 /*static inline*/ s_time_t ticks_to_ns(uint64_t ticks)
@@ -120,15 +120,17 @@ int __init init_xen_time(void)
     /* Retrieve all IRQs for the timer */
     for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
     {
-        res = dt_device_get_irq(dev, i, &timer_irq[i]);
-        if ( res )
+        res = platform_get_irq(dev, i);
+
+        if ( res < 0 )
             panic("Timer: Unable to retrieve IRQ %u from the device tree", i);
+        timer_irq[i] = res;
     }
 
     printk("Generic Timer IRQ: phys=%u hyp=%u virt=%u\n",
-           timer_irq[TIMER_PHYS_NONSECURE_PPI].irq,
-           timer_irq[TIMER_HYP_PPI].irq,
-           timer_irq[TIMER_VIRT_PPI].irq);
+           timer_irq[TIMER_PHYS_NONSECURE_PPI],
+           timer_irq[TIMER_HYP_PPI],
+           timer_irq[TIMER_VIRT_PPI]);
 
     res = platform_init_time();
     if ( res )
@@ -192,7 +194,7 @@ int reprogram_timer(s_time_t timeout)
 /* Handle the firing timer */
 static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
 {
-    if ( irq == (timer_irq[TIMER_HYP_PPI].irq) &&
+    if ( irq == (timer_irq[TIMER_HYP_PPI]) &&
          READ_SYSREG32(CNTHP_CTL_EL2) & CNTx_CTL_PENDING )
     {
         /* Signal the generic timer code to do its work */
@@ -201,7 +203,7 @@ static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
         WRITE_SYSREG32(0, CNTHP_CTL_EL2);
     }
 
-    if ( irq == (timer_irq[TIMER_PHYS_NONSECURE_PPI].irq) &&
+    if ( irq == (timer_irq[TIMER_PHYS_NONSECURE_PPI]) &&
          READ_SYSREG32(CNTP_CTL_EL0) & CNTx_CTL_PENDING )
     {
         /* Signal the generic timer code to do its work */
@@ -235,12 +237,12 @@ void __cpuinit init_timer_interrupt(void)
     WRITE_SYSREG32(0, CNTHP_CTL_EL2);   /* Hypervisor's timer disabled */
     isb();
 
-    request_dt_irq(&timer_irq[TIMER_HYP_PPI], timer_interrupt,
-                   "hyptimer", NULL);
-    request_dt_irq(&timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
+    request_irq(timer_irq[TIMER_HYP_PPI], timer_interrupt,
+                "hyptimer", NULL);
+    request_irq(timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
                    "virtimer", NULL);
-    request_dt_irq(&timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
-                   "phytimer", NULL);
+    request_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
+                "phytimer", NULL);
 }
 
 /* Wait a set number of microseconds */
diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index 370539c..74ee578 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -30,7 +30,7 @@
 
 static struct exynos4210_uart {
     unsigned int baud, clock_hz, data_bits, parity, stop_bits;
-    struct dt_irq irq;
+    unsigned int irq;
     void *regs;
     struct irqaction irqaction;
     struct vuart_info vuart;
@@ -197,9 +197,9 @@ static void __init exynos4210_uart_init_postirq(struct serial_port *port)
     uart->irqaction.name    = "exynos4210_uart";
     uart->irqaction.dev_id  = port;
 
-    if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
+    if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
         dprintk(XENLOG_ERR, "Failed to allocated exynos4210_uart IRQ %d\n",
-                uart->irq.irq);
+                uart->irq);
 
     /* Unmask interrupts */
     exynos4210_write(uart, UINTM, ~UINTM_ALLI);
@@ -272,7 +272,7 @@ static int __init exynos4210_uart_irq(struct serial_port *port)
 {
     struct exynos4210_uart *uart = port->uart;
 
-    return uart->irq.irq;
+    return uart->irq;
 }
 
 static const struct vuart_info *exynos4210_vuart_info(struct serial_port *port)
@@ -323,12 +323,13 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
         return res;
     }
 
-    res = dt_device_get_irq(dev, 0, &uart->irq);
-    if ( res )
+    res = platform_get_irq(dev, 0);
+    if ( uart->irq < 0 )
     {
         printk("exynos4210: Unable to retrieve the IRQ\n");
-        return res;
+        return -EINVAL;
     }
+    uart->irq = res;
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 21f086a..6691806 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -76,9 +76,6 @@ static struct ns16550 {
     u8 bar_idx;
     bool_t enable_ro; /* Make MMIO devices read only to Dom0 */
 #endif
-#ifdef HAS_DEVICE_TREE
-    struct dt_irq dt_irq;
-#endif
 } ns16550_com[2] = { { 0 } };
 
 struct ns16550_config_mmio {
@@ -612,13 +609,8 @@ static void __init ns16550_init_postirq(struct serial_port *port)
         uart->irqaction.handler = ns16550_interrupt;
         uart->irqaction.name    = "ns16550";
         uart->irqaction.dev_id  = port;
-#ifdef HAS_DEVICE_TREE
-        if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0 )
-            printk("ERROR: Failed to allocate ns16550 DT IRQ.\n");
-#else
         if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
             printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
-#endif
     }
 
     ns16550_setup_postirq(uart);
@@ -1172,12 +1164,10 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
     if ( uart->reg_width != 1 && uart->reg_width != 4 )
         return -EINVAL;
 
-    res = dt_device_get_irq(dev, 0, &uart->dt_irq);
-    if ( res )
-        return res;
-
-    /* The common bit of the driver mostly deals with irq not dt_irq. */
-    uart->irq = uart->dt_irq.irq;
+    res = platform_get_irq(dev, 0);
+    if ( ! res )
+        return -EINVAL;
+    uart->irq = res;
 
     uart->dw_usr_bsy = dt_device_is_compatible(dev, "snps,dw-apb-uart");
 
diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index b8da509..e598785 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -30,7 +30,7 @@
 
 static struct omap_uart {
     u32 baud, clock_hz, data_bits, parity, stop_bits, fifo_size;
-    struct dt_irq irq;
+    unsigned int irq;
     char __iomem *regs;
     struct irqaction irqaction;
     struct vuart_info vuart;
@@ -205,10 +205,10 @@ static void __init omap_uart_init_postirq(struct serial_port *port)
     uart->irqaction.name = "omap_uart";
     uart->irqaction.dev_id = port;
 
-    if ( setup_dt_irq(&uart->irq, &uart->irqaction) != 0 )
+    if ( setup_irq(uart->irq, &uart->irqaction) != 0 )
     {
         dprintk(XENLOG_ERR, "Failed to allocated omap_uart IRQ %d\n",
-                uart->irq.irq);
+                uart->irq);
         return;
     }
 
@@ -259,7 +259,7 @@ static int __init omap_uart_irq(struct serial_port *port)
 {
     struct omap_uart *uart = port->uart;
 
-    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
+    return ((uart->irq > 0) ? uart->irq : -1);
 }
 
 static const struct vuart_info *omap_vuart_info(struct serial_port *port)
@@ -317,12 +317,13 @@ static int __init omap_uart_init(struct dt_device_node *dev,
         return res;
     }
 
-    res = dt_device_get_irq(dev, 0, &uart->irq);
-    if ( res )
+    res = platform_get_irq(dev, 0);
+    if ( res < 0 )
     {
         printk("omap-uart: Unable to retrieve the IRQ\n");
-        return res;
+        return -EINVAL;
     }
+    uart->irq = res;
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 459e686..89bda94 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -32,7 +32,7 @@
 
 static struct pl011 {
     unsigned int baud, clock_hz, data_bits, parity, stop_bits;
-    struct dt_irq irq;
+    unsigned int irq;
     void __iomem *regs;
     /* UART with IRQ line: interrupt-driven I/O. */
     struct irqaction irqaction;
@@ -132,13 +132,13 @@ static void __init pl011_init_postirq(struct serial_port *port)
     struct pl011 *uart = port->uart;
     int rc;
 
-    if ( uart->irq.irq > 0 )
+    if ( uart->irq > 0 )
     {
         uart->irqaction.handler = pl011_interrupt;
         uart->irqaction.name    = "pl011";
         uart->irqaction.dev_id  = port;
-        if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
-            printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq.irq);
+        if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+            printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq);
     }
 
     /* Clear pending error interrupts */
@@ -186,7 +186,8 @@ static int pl011_getc(struct serial_port *port, char *pc)
 static int __init pl011_irq(struct serial_port *port)
 {
     struct pl011 *uart = port->uart;
-    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
+
+    return ((uart->irq > 0) ? uart->irq : -1);
 }
 
 static const struct vuart_info *pl011_vuart(struct serial_port *port)
@@ -239,12 +240,13 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         return res;
     }
 
-    res = dt_device_get_irq(dev, 0, &uart->irq);
-    if ( res )
+    res = platform_get_irq(dev, 0);
+    if ( res < 0 )
     {
         printk("pl011: Unable to retrieve the IRQ\n");
-        return res;
+        return -EINVAL;
     }
+    uart->irq = res;
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 93d2128..f9b9a9f 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -40,11 +40,6 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq);
 void init_IRQ(void);
 void init_secondary_IRQ(void);
 
-int request_dt_irq(const struct dt_irq *irq,
-                   void (*handler)(int, void *, struct cpu_user_regs *),
-                   const char *devname, void *dev_id);
-int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new);
-
 int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
                           const char *devname);
 int platform_get_irq(const struct dt_device_node *device,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 16/18] xen: IRQ: Add dev_id parameter to release_irq
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (14 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 15/18] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 17/18] xen/arm: IRQ: extend {request, setup}_irq to take an irqflags in parameter Julien Grall
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel
  Cc: stefano.stabellini, Julien Grall, tim, ian.campbell, Jan Beulich

The new parameter (dev_id) will be used in on ARM to release the right
action when support for multiple action is added.

Even if this function is declared in common code, no one is using it. So it's
safe to modify the prototype also for x86.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

---
    Changes in v3:
        - Fix typoes in commit message
        - Don't remove __init on release_irq for x86

    Changes in v2:
        - Patch added
---
 xen/arch/arm/irq.c    |    2 +-
 xen/arch/x86/irq.c    |    2 +-
 xen/include/xen/irq.h |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index a09ca25..45fbadb 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -216,7 +216,7 @@ out_no_end:
     irq_exit();
 }
 
-void release_irq(unsigned int irq)
+void release_irq(unsigned int irq, const void *dev_id)
 {
     struct irq_desc *desc;
     unsigned long flags;
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 5b5b169..727472d 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -983,7 +983,7 @@ int __init request_irq(unsigned int irq,
     return retval;
 }
 
-void __init release_irq(unsigned int irq)
+void __init release_irq(unsigned int irq, const void *dev_id)
 {
     struct irq_desc *desc;
     unsigned long flags;
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index f2e6215..1f8bdb3 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -90,7 +90,7 @@ int arch_init_one_irq_desc(struct irq_desc *);
 #define irq_desc_initialized(desc) ((desc)->handler != NULL)
 
 extern int setup_irq(unsigned int irq, struct irqaction *);
-extern void release_irq(unsigned int irq);
+extern void release_irq(unsigned int irq, const void *dev_id);
 extern int request_irq(unsigned int irq,
                void (*handler)(int, void *, struct cpu_user_regs *),
                const char * devname, void *dev_id);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 17/18] xen/arm: IRQ: extend {request, setup}_irq to take an irqflags in parameter
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (15 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 16/18] xen: IRQ: Add dev_id parameter to release_irq Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 12:58 ` [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ Julien Grall
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel
  Cc: Keir Fraser, ian.campbell, Julien Grall, tim, stefano.stabellini,
	Jan Beulich, Suravee Suthikulpanit, Xiantao Zhang

The irqflags will be used later on ARM to know if we can shared the IRQ or not.

On x86, the irqflags should always be 0.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Xiantao Zhang <xiantao.zhang@intel.com>

---
    Changes in v4:
        - request_irq should pass irqflags to setup irq on x86

    Changes in v3:
        - Patch added
---
 xen/arch/arm/gic.c                       |    2 +-
 xen/arch/arm/irq.c                       |    6 +++---
 xen/arch/arm/time.c                      |    6 +++---
 xen/arch/x86/hpet.c                      |    2 +-
 xen/arch/x86/i8259.c                     |    2 +-
 xen/arch/x86/irq.c                       |    9 ++++++---
 xen/arch/x86/time.c                      |    2 +-
 xen/drivers/char/exynos4210-uart.c       |    2 +-
 xen/drivers/char/ns16550.c               |    2 +-
 xen/drivers/char/omap-uart.c             |    2 +-
 xen/drivers/char/pl011.c                 |    2 +-
 xen/drivers/passthrough/amd/iommu_init.c |    2 +-
 xen/drivers/passthrough/vtd/iommu.c      |    2 +-
 xen/include/xen/irq.h                    |    5 +++--
 14 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index cb40b0a..39f9621 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -885,7 +885,7 @@ void gic_dump_info(struct vcpu *v)
 
 void __cpuinit init_maintenance_interrupt(void)
 {
-    request_irq(gic.maintenance_irq, maintenance_interrupt,
+    request_irq(gic.maintenance_irq, 0, maintenance_interrupt,
                 "irq-maintenance", NULL);
 }
 
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 45fbadb..1bc960d 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -117,7 +117,7 @@ static inline struct domain *irq_get_domain(struct irq_desc *desc)
     return desc->action->dev_id;
 }
 
-int request_irq(unsigned int irq,
+int request_irq(unsigned int irq, unsigned int irqflags,
                 void (*handler)(int, void *, struct cpu_user_regs *),
                 const char *devname, void *dev_id)
 {
@@ -144,7 +144,7 @@ int request_irq(unsigned int irq,
     action->dev_id = dev_id;
     action->free_on_release = 1;
 
-    retval = setup_irq(irq, action);
+    retval = setup_irq(irq, irqflags, action);
     if ( retval )
         xfree(action);
 
@@ -252,7 +252,7 @@ static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
     return 0;
 }
 
-int setup_irq(unsigned int irq, struct irqaction *new)
+int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new)
 {
     int rc;
     unsigned long flags;
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 0bff747..81452b5 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -237,11 +237,11 @@ void __cpuinit init_timer_interrupt(void)
     WRITE_SYSREG32(0, CNTHP_CTL_EL2);   /* Hypervisor's timer disabled */
     isb();
 
-    request_irq(timer_irq[TIMER_HYP_PPI], timer_interrupt,
+    request_irq(timer_irq[TIMER_HYP_PPI], 0, timer_interrupt,
                 "hyptimer", NULL);
-    request_irq(timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
+    request_irq(timer_irq[TIMER_VIRT_PPI], 0, vtimer_interrupt,
                    "virtimer", NULL);
-    request_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
+    request_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], 0, timer_interrupt,
                 "phytimer", NULL);
 }
 
diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 3a4f7e8..0b13f52 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -355,7 +355,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
     hpet_write32(cfg, HPET_Tn_CFG(ch->idx));
 
     desc->handler = &hpet_msi_type;
-    ret = request_irq(ch->msi.irq, hpet_interrupt_handler, "HPET", ch);
+    ret = request_irq(ch->msi.irq, 0, hpet_interrupt_handler, "HPET", ch);
     if ( ret >= 0 )
         ret = __hpet_setup_msi_irq(desc);
     if ( ret < 0 )
diff --git a/xen/arch/x86/i8259.c b/xen/arch/x86/i8259.c
index 6fdcce8..a0270c9 100644
--- a/xen/arch/x86/i8259.c
+++ b/xen/arch/x86/i8259.c
@@ -435,6 +435,6 @@ void __init init_IRQ(void)
     outb_p(LATCH & 0xff, PIT_CH0); /* LSB */
     outb(LATCH >> 8, PIT_CH0);     /* MSB */
 
-    setup_irq(2, &cascade);
+    setup_irq(2, 0, &cascade);
 }
 
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 727472d..dafd338 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -949,7 +949,7 @@ static int __init irq_ratelimit_init(void)
 }
 __initcall(irq_ratelimit_init);
 
-int __init request_irq(unsigned int irq,
+int __init request_irq(unsigned int irq, unsigned int irqflags,
         void (*handler)(int, void *, struct cpu_user_regs *),
         const char * devname, void *dev_id)
 {
@@ -976,7 +976,7 @@ int __init request_irq(unsigned int irq,
     action->dev_id = dev_id;
     action->free_on_release = 1;
 
-    retval = setup_irq(irq, action);
+    retval = setup_irq(irq, irqflags, action);
     if (retval)
         xfree(action);
 
@@ -1005,11 +1005,14 @@ void __init release_irq(unsigned int irq, const void *dev_id)
         xfree(action);
 }
 
-int __init setup_irq(unsigned int irq, struct irqaction *new)
+int __init setup_irq(unsigned int irq, unsigned int irqflags,
+                     struct irqaction *new)
 {
     struct irq_desc *desc;
     unsigned long flags;
 
+    ASSERT(irqflags == 0);
+
     desc = irq_to_desc(irq);
  
     spin_lock_irqsave(&desc->lock,flags);
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 93bb5b6..c339f92 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1473,7 +1473,7 @@ void __init early_time_init(void)
     printk("Detected %lu.%03lu MHz processor.\n", 
            cpu_khz / 1000, cpu_khz % 1000);
 
-    setup_irq(0, &irq0);
+    setup_irq(0, 0, &irq0);
 }
 
 /* keep pit enabled for pit_broadcast working while cpuidle enabled */
diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index 74ee578..76c0d7e 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -197,7 +197,7 @@ static void __init exynos4210_uart_init_postirq(struct serial_port *port)
     uart->irqaction.name    = "exynos4210_uart";
     uart->irqaction.dev_id  = port;
 
-    if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+    if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
         dprintk(XENLOG_ERR, "Failed to allocated exynos4210_uart IRQ %d\n",
                 uart->irq);
 
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 6691806..161b251 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -609,7 +609,7 @@ static void __init ns16550_init_postirq(struct serial_port *port)
         uart->irqaction.handler = ns16550_interrupt;
         uart->irqaction.name    = "ns16550";
         uart->irqaction.dev_id  = port;
-        if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+        if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
             printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
     }
 
diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index e598785..a798b8d 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -205,7 +205,7 @@ static void __init omap_uart_init_postirq(struct serial_port *port)
     uart->irqaction.name = "omap_uart";
     uart->irqaction.dev_id = port;
 
-    if ( setup_irq(uart->irq, &uart->irqaction) != 0 )
+    if ( setup_irq(uart->irq, 0, &uart->irqaction) != 0 )
     {
         dprintk(XENLOG_ERR, "Failed to allocated omap_uart IRQ %d\n",
                 uart->irq);
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 89bda94..dd19ce8 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -137,7 +137,7 @@ static void __init pl011_init_postirq(struct serial_port *port)
         uart->irqaction.handler = pl011_interrupt;
         uart->irqaction.name    = "pl011";
         uart->irqaction.dev_id  = port;
-        if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+        if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
             printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq);
     }
 
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index 4686813..6ae44d9 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -815,7 +815,7 @@ static bool_t __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
         handler = &iommu_msi_type;
     ret = __setup_msi_irq(irq_to_desc(irq), &iommu->msi, handler);
     if ( !ret )
-        ret = request_irq(irq, iommu_interrupt_handler, "amd_iommu", iommu);
+        ret = request_irq(irq, 0, iommu_interrupt_handler, "amd_iommu", iommu);
     if ( ret )
     {
         destroy_irq(irq);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index abaa8c9..c94b0d1 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1082,7 +1082,7 @@ static int __init iommu_set_interrupt(struct acpi_drhd_unit *drhd)
 
     desc = irq_to_desc(irq);
     desc->handler = &dma_msi_type;
-    ret = request_irq(irq, iommu_page_fault, "dmar", iommu);
+    ret = request_irq(irq, 0, iommu_page_fault, "dmar", iommu);
     if ( ret )
     {
         desc->handler = &no_irq_type;
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index 1f8bdb3..f9a18d8 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -89,9 +89,10 @@ int arch_init_one_irq_desc(struct irq_desc *);
 
 #define irq_desc_initialized(desc) ((desc)->handler != NULL)
 
-extern int setup_irq(unsigned int irq, struct irqaction *);
+extern int setup_irq(unsigned int irq, unsigned int irqflags,
+                     struct irqaction *);
 extern void release_irq(unsigned int irq, const void *dev_id);
-extern int request_irq(unsigned int irq,
+extern int request_irq(unsigned int irq, unsigned int irqflags,
                void (*handler)(int, void *, struct cpu_user_regs *),
                const char * devname, void *dev_id);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (16 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 17/18] xen/arm: IRQ: extend {request, setup}_irq to take an irqflags in parameter Julien Grall
@ 2014-04-22 12:58 ` Julien Grall
  2014-04-22 13:36   ` Jan Beulich
  2014-04-23 14:07   ` Ian Campbell
  2014-05-02  8:52 ` [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
  2014-05-02 12:51 ` Ian Campbell
  19 siblings, 2 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 12:58 UTC (permalink / raw)
  To: xen-devel
  Cc: Keir Fraser, ian.campbell, Julien Grall, tim, stefano.stabellini,
	Jan Beulich

On ARM, it may happen (eg ARM SMMU) to setup multiple handler for the same
interrupt.

To be able to use multiple action, the driver has to explicitly call
{setup,request}_irq with IRQF_SHARED as 2nd parameter.

The behavior stays the same on x86, e.g only one action is handled.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>

---
    Changes in v4:
        - Go back to a single custom linked list. The double linked-list
        doesn't fit the requirements (i.e browsing safely without look) and
        the llist.h from Linux doesn't allow use to delete a node in the middle
        of the list.

    Changes in v3:
        - Drop {setup,request}_irq_flags, the current function has been
        extended on an earlier patch.
        - Rename IRQ_SHARED into IRQF_SHARED

    Changes in v2:
        - Explain design choice
        - Introduce CONFIG_IRQ_HAS_MULTIPLE_ACTION
        - Use list instead of custom pointer
        - release_irq should not shutdown the IRQ at the beginning
        - Add IRQ_SHARED flags
        - Introduce request_irq_flags and setup_irq_flags
        - Fix compilation on x86. Some code are creating the irqaction
        via = { ... } so the "next" field should be moved
        toward the end
---
 xen/arch/arm/irq.c           |   79 +++++++++++++++++++++++++++++++++---------
 xen/include/asm-arm/config.h |    2 ++
 xen/include/xen/irq.h        |    4 +++
 3 files changed, 68 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 1bc960d..d05c0be 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -155,7 +155,6 @@ int request_irq(unsigned int irq, unsigned int irqflags,
 void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
 {
     struct irq_desc *desc = irq_to_desc(irq);
-    struct irqaction *action = desc->action;
 
     /* TODO: perfc_incr(irqs); */
 
@@ -166,7 +165,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
     spin_lock(&desc->lock);
     desc->handler->ack(desc);
 
-    if ( action == NULL )
+    if ( !desc->action )
     {
         printk("Unknown %s %#3.3x\n",
                is_fiq ? "FIQ" : "IRQ", irq);
@@ -198,12 +197,21 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
 
     desc->status |= IRQ_INPROGRESS;
 
-    action = desc->action;
     while ( desc->status & IRQ_PENDING )
     {
+        struct irqaction *action;
+
         desc->status &= ~IRQ_PENDING;
+        action = desc->action;
+
         spin_unlock_irq(&desc->lock);
-        action->handler(irq, action->dev_id, regs);
+
+        do
+        {
+            action->handler(irq, action->dev_id, regs);
+            action = action->next;
+        } while ( action );
+
         spin_lock_irq(&desc->lock);
     }
 
@@ -220,34 +228,71 @@ void release_irq(unsigned int irq, const void *dev_id)
 {
     struct irq_desc *desc;
     unsigned long flags;
-   struct irqaction *action;
+    struct irqaction *action, **action_ptr;
 
     desc = irq_to_desc(irq);
 
     spin_lock_irqsave(&desc->lock,flags);
 
-    desc->handler->shutdown(desc);
+    action_ptr = &desc->action;
+    for ( ;; )
+    {
+        action = *action_ptr;
+        if ( !action )
+        {
+            printk(XENLOG_WARNING "Trying to free already-free IRQ %u\n", irq);
+            spin_unlock_irqrestore(&desc->lock, flags);
+            return;
+        }
+
+        if ( action->dev_id == dev_id )
+            break;
+
+        action_ptr = &action->next;
+    }
+
+    /* Found it - remove it from the action list */
+    *action_ptr = action->next;
 
-    action = desc->action;
-    desc->action  = NULL;
-    desc->status &= ~IRQ_GUEST;
+    /* If this was the last action, shut down the IRQ */
+    if ( !desc->action )
+    {
+        desc->handler->shutdown(desc);
+        desc->status &= ~IRQ_GUEST;
+    }
 
     spin_unlock_irqrestore(&desc->lock,flags);
 
     /* Wait to make sure it's not being used on another CPU */
     do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
 
-    if ( action && action->free_on_release )
+    if ( action->free_on_release )
         xfree(action);
 }
 
-static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
+static int __setup_irq(struct irq_desc *desc, unsigned int irqflags,
+                       struct irqaction *new)
 {
-    if ( desc->action != NULL )
-        return -EBUSY;
+    bool_t shared = !!(irqflags & IRQF_SHARED);
+
+    ASSERT(new != NULL);
+
+    /* Sanity checks:
+     *  - if the IRQ is marked as shared
+     *  - dev_id is not NULL when IRQF_SHARED is set
+     */
+    if ( desc->action != NULL && (!(desc->status & IRQF_SHARED) || !shared) )
+        return -EINVAL;
+    if ( shared && new->dev_id == NULL )
+        return -EINVAL;
+
+    if ( shared )
+        desc->status |= IRQF_SHARED;
 
-    desc->action  = new;
-    dsb(sy);
+    new->next = desc->action;
+    dsb(ish);
+    desc->action = new;
+    dsb(ish);
 
     return 0;
 }
@@ -275,7 +320,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new)
 
     disabled = (desc->action == NULL);
 
-    rc = __setup_irq(desc, new);
+    rc = __setup_irq(desc, irqflags, new);
     if ( rc )
         goto err;
 
@@ -340,7 +385,7 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
         goto out;
     }
 
-    retval = __setup_irq(desc, action);
+    retval = __setup_irq(desc, 0, action);
     if ( retval )
         goto out;
 
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index ef291ff..1c3abcf 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -37,6 +37,8 @@
 
 #define CONFIG_VIDEO 1
 
+#define CONFIG_IRQ_HAS_MULTIPLE_ACTION 1
+
 #define OPT_CONSOLE_STR "dtuart"
 
 #ifdef MAX_PHYS_CPUS
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index f9a18d8..40c0f3f 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -14,6 +14,9 @@ struct irqaction {
     const char *name;
     void *dev_id;
     bool_t free_on_release;
+#ifdef CONFIG_IRQ_HAS_MULTIPLE_ACTION
+    struct irqaction *next;
+#endif
 };
 
 /*
@@ -27,6 +30,7 @@ struct irqaction {
 #define IRQ_MOVE_PENDING  (1u<<5) /* IRQ is migrating to another CPUs */
 #define IRQ_PER_CPU       (1u<<6) /* IRQ is per CPU */
 #define IRQ_GUEST_EOI_PENDING (1u<<7) /* IRQ was disabled, pending a guest EOI */
+#define IRQF_SHARED       (1<<8)  /* IRQ is shared */
 
 /* Special IRQ numbers. */
 #define AUTO_ASSIGN_IRQ         (-1)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 15/18] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq
  2014-04-22 12:58 ` [PATCH v4 15/18] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq Julien Grall
@ 2014-04-22 13:26   ` Julien Grall
  0 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-22 13:26 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Keir Fraser, tim, ian.campbell, stefano.stabellini

On 04/22/2014 01:58 PM, Julien Grall wrote:
> diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
> index 370539c..74ee578 100644
> --- a/xen/drivers/char/exynos4210-uart.c
> +++ b/xen/drivers/char/exynos4210-uart.c
> @@ -30,7 +30,7 @@

>  static const struct vuart_info *exynos4210_vuart_info(struct serial_port *port)
> @@ -323,12 +323,13 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
>          return res;
>      }
>  
> -    res = dt_device_get_irq(dev, 0, &uart->irq);
> -    if ( res )
> +    res = platform_get_irq(dev, 0);
> +    if ( uart->irq < 0 )

Argh, I test the wrong variable here. This should be

if ( res < 0 )

I've copied the slightly modify patch below.

commit 7a98180798deed5e453128bc3556cadef025d894
Author: Julien Grall <julien.grall@linaro.org>
Date:   Thu Mar 27 17:54:27 2014 +0000

    xen/arm: IRQ: Replace {request,setup}_dt_irq by {request,setup}_irq
    
    Now that irq_desc stores the type of the IRQ (e.g level/edge,...), we don't
    need to use specific IRQ function for ARM.
    
    Also replace every call to dt_device_get_irq by platform_get_irq which is
    a wrapper to this function and setup the IRQ type correctly.
    
    Signed-off-by: Julien Grall <julien.grall@linaro.org>
    Acked-by: Ian Campbell <ian.campbell@citrix.com>
    Cc: Keir Fraser <keir@xen.org>
    
    ---
        Changes in v5:
            - exynos4210 was testing the wrong IRQ variable
    
        Changes in v4:
            - platform_get_irq now returns -1 in case of error.
    
        Changes in v3:
            - Fix typoes in commit message
    
        Changes in v2:
            - Patch added

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 4a6ed35..cb40b0a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -47,7 +47,7 @@ static struct {
     paddr_t hbase;       /* Address of virtual interface registers */
     paddr_t vbase;       /* Address of virtual cpu interface registers */
     unsigned int lines;  /* Number of interrupts (SPIs + PPIs + SGIs) */
-    struct dt_irq maintenance; /* IRQ maintenance */
+    unsigned int maintenance_irq; /* IRQ maintenance */
     unsigned int cpus;
     spinlock_t lock;
 } gic;
@@ -431,9 +431,10 @@ void __init gic_init(void)
     if ( res || !gic.vbase || (gic.vbase & ~PAGE_MASK) )
         panic("GIC: Cannot find a valid address for the virtual CPU");
 
-    res = dt_device_get_irq(node, 0, &gic.maintenance);
-    if ( res )
+    res = platform_get_irq(node, 0);
+    if ( res < 0 )
         panic("GIC: Cannot find the maintenance IRQ");
+    gic.maintenance_irq = res;
 
     /* Set the GIC as the primary interrupt controller */
     dt_interrupt_controller = node;
@@ -447,7 +448,7 @@ void __init gic_init(void)
               "        gic_vcpu_addr=%"PRIpaddr"\n"
               "        gic_maintenance_irq=%u\n",
               gic.dbase, gic.cbase, gic.hbase, gic.vbase,
-              gic.maintenance.irq);
+              gic.maintenance_irq);
 
     if ( (gic.dbase & ~PAGE_MASK) || (gic.cbase & ~PAGE_MASK) ||
          (gic.hbase & ~PAGE_MASK) || (gic.vbase & ~PAGE_MASK) )
@@ -884,8 +885,8 @@ void gic_dump_info(struct vcpu *v)
 
 void __cpuinit init_maintenance_interrupt(void)
 {
-    request_dt_irq(&gic.maintenance, maintenance_interrupt,
-                   "irq-maintenance", NULL);
+    request_irq(gic.maintenance_irq, maintenance_interrupt,
+                "irq-maintenance", NULL);
 }
 
 /*
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index f1d6398..a09ca25 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -117,9 +117,9 @@ static inline struct domain *irq_get_domain(struct irq_desc *desc)
     return desc->action->dev_id;
 }
 
-int request_dt_irq(const struct dt_irq *irq,
-                   void (*handler)(int, void *, struct cpu_user_regs *),
-                   const char *devname, void *dev_id)
+int request_irq(unsigned int irq,
+                void (*handler)(int, void *, struct cpu_user_regs *),
+                const char *devname, void *dev_id)
 {
     struct irqaction *action;
     int retval;
@@ -130,13 +130,13 @@ int request_dt_irq(const struct dt_irq *irq,
      * which interrupt is which (messes up the interrupt freeing
      * logic etc).
      */
-    if (irq->irq >= nr_irqs)
+    if ( irq >= nr_irqs )
         return -EINVAL;
-    if (!handler)
+    if ( !handler )
         return -EINVAL;
 
     action = xmalloc(struct irqaction);
-    if (!action)
+    if ( !action )
         return -ENOMEM;
 
     action->handler = handler;
@@ -144,8 +144,8 @@ int request_dt_irq(const struct dt_irq *irq,
     action->dev_id = dev_id;
     action->free_on_release = 1;
 
-    retval = setup_dt_irq(irq, action);
-    if (retval)
+    retval = setup_irq(irq, action);
+    if ( retval )
         xfree(action);
 
     return retval;
@@ -252,14 +252,14 @@ static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
     return 0;
 }
 
-int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
+int setup_irq(unsigned int irq, struct irqaction *new)
 {
     int rc;
     unsigned long flags;
     struct irq_desc *desc;
     bool_t disabled;
 
-    desc = irq_to_desc(irq->irq);
+    desc = irq_to_desc(irq);
 
     spin_lock_irqsave(&desc->lock, flags);
 
@@ -269,7 +269,7 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
 
         spin_unlock_irqrestore(&desc->lock, flags);
         printk(XENLOG_ERR "ERROR: IRQ %u is already in use by the domain %u\n",
-               irq->irq, d->domain_id);
+               irq, d->domain_id);
         return -EBUSY;
     }
 
@@ -289,7 +289,6 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
          * TODO: Handle case where SPI is setup on different CPU than
          * the targeted CPU and the priority.
          */
-        desc->arch.type = irq->type;
         gic_route_irq_to_xen(desc, cpumask_of(smp_processor_id()),
                              GIC_PRI_IRQ);
         desc->handler->startup(desc);
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index ce96337..0bff747 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -48,13 +48,13 @@ uint64_t __read_mostly boot_count;
  * register-mapped time source in the SoC. */
 unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
 
-static struct dt_irq timer_irq[MAX_TIMER_PPI];
+static unsigned int timer_irq[MAX_TIMER_PPI];
 
 unsigned int timer_get_irq(enum timer_ppi ppi)
 {
     ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
 
-    return timer_irq[ppi].irq;
+    return timer_irq[ppi];
 }
 
 /*static inline*/ s_time_t ticks_to_ns(uint64_t ticks)
@@ -120,15 +120,17 @@ int __init init_xen_time(void)
     /* Retrieve all IRQs for the timer */
     for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
     {
-        res = dt_device_get_irq(dev, i, &timer_irq[i]);
-        if ( res )
+        res = platform_get_irq(dev, i);
+
+        if ( res < 0 )
             panic("Timer: Unable to retrieve IRQ %u from the device tree", i);
+        timer_irq[i] = res;
     }
 
     printk("Generic Timer IRQ: phys=%u hyp=%u virt=%u\n",
-           timer_irq[TIMER_PHYS_NONSECURE_PPI].irq,
-           timer_irq[TIMER_HYP_PPI].irq,
-           timer_irq[TIMER_VIRT_PPI].irq);
+           timer_irq[TIMER_PHYS_NONSECURE_PPI],
+           timer_irq[TIMER_HYP_PPI],
+           timer_irq[TIMER_VIRT_PPI]);
 
     res = platform_init_time();
     if ( res )
@@ -192,7 +194,7 @@ int reprogram_timer(s_time_t timeout)
 /* Handle the firing timer */
 static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
 {
-    if ( irq == (timer_irq[TIMER_HYP_PPI].irq) &&
+    if ( irq == (timer_irq[TIMER_HYP_PPI]) &&
          READ_SYSREG32(CNTHP_CTL_EL2) & CNTx_CTL_PENDING )
     {
         /* Signal the generic timer code to do its work */
@@ -201,7 +203,7 @@ static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
         WRITE_SYSREG32(0, CNTHP_CTL_EL2);
     }
 
-    if ( irq == (timer_irq[TIMER_PHYS_NONSECURE_PPI].irq) &&
+    if ( irq == (timer_irq[TIMER_PHYS_NONSECURE_PPI]) &&
          READ_SYSREG32(CNTP_CTL_EL0) & CNTx_CTL_PENDING )
     {
         /* Signal the generic timer code to do its work */
@@ -235,12 +237,12 @@ void __cpuinit init_timer_interrupt(void)
     WRITE_SYSREG32(0, CNTHP_CTL_EL2);   /* Hypervisor's timer disabled */
     isb();
 
-    request_dt_irq(&timer_irq[TIMER_HYP_PPI], timer_interrupt,
-                   "hyptimer", NULL);
-    request_dt_irq(&timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
+    request_irq(timer_irq[TIMER_HYP_PPI], timer_interrupt,
+                "hyptimer", NULL);
+    request_irq(timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
                    "virtimer", NULL);
-    request_dt_irq(&timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
-                   "phytimer", NULL);
+    request_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
+                "phytimer", NULL);
 }
 
 /* Wait a set number of microseconds */
diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index 370539c..404ce05 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -30,7 +30,7 @@
 
 static struct exynos4210_uart {
     unsigned int baud, clock_hz, data_bits, parity, stop_bits;
-    struct dt_irq irq;
+    unsigned int irq;
     void *regs;
     struct irqaction irqaction;
     struct vuart_info vuart;
@@ -197,9 +197,9 @@ static void __init exynos4210_uart_init_postirq(struct serial_port *port)
     uart->irqaction.name    = "exynos4210_uart";
     uart->irqaction.dev_id  = port;
 
-    if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
+    if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
         dprintk(XENLOG_ERR, "Failed to allocated exynos4210_uart IRQ %d\n",
-                uart->irq.irq);
+                uart->irq);
 
     /* Unmask interrupts */
     exynos4210_write(uart, UINTM, ~UINTM_ALLI);
@@ -272,7 +272,7 @@ static int __init exynos4210_uart_irq(struct serial_port *port)
 {
     struct exynos4210_uart *uart = port->uart;
 
-    return uart->irq.irq;
+    return uart->irq;
 }
 
 static const struct vuart_info *exynos4210_vuart_info(struct serial_port *port)
@@ -323,12 +323,13 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
         return res;
     }
 
-    res = dt_device_get_irq(dev, 0, &uart->irq);
-    if ( res )
+    res = platform_get_irq(dev, 0);
+    if ( res < 0 )
     {
         printk("exynos4210: Unable to retrieve the IRQ\n");
-        return res;
+        return -EINVAL;
     }
+    uart->irq = res;
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 21f086a..6691806 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -76,9 +76,6 @@ static struct ns16550 {
     u8 bar_idx;
     bool_t enable_ro; /* Make MMIO devices read only to Dom0 */
 #endif
-#ifdef HAS_DEVICE_TREE
-    struct dt_irq dt_irq;
-#endif
 } ns16550_com[2] = { { 0 } };
 
 struct ns16550_config_mmio {
@@ -612,13 +609,8 @@ static void __init ns16550_init_postirq(struct serial_port *port)
         uart->irqaction.handler = ns16550_interrupt;
         uart->irqaction.name    = "ns16550";
         uart->irqaction.dev_id  = port;
-#ifdef HAS_DEVICE_TREE
-        if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0 )
-            printk("ERROR: Failed to allocate ns16550 DT IRQ.\n");
-#else
         if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
             printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
-#endif
     }
 
     ns16550_setup_postirq(uart);
@@ -1172,12 +1164,10 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
     if ( uart->reg_width != 1 && uart->reg_width != 4 )
         return -EINVAL;
 
-    res = dt_device_get_irq(dev, 0, &uart->dt_irq);
-    if ( res )
-        return res;
-
-    /* The common bit of the driver mostly deals with irq not dt_irq. */
-    uart->irq = uart->dt_irq.irq;
+    res = platform_get_irq(dev, 0);
+    if ( ! res )
+        return -EINVAL;
+    uart->irq = res;
 
     uart->dw_usr_bsy = dt_device_is_compatible(dev, "snps,dw-apb-uart");
 
diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index b8da509..e598785 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -30,7 +30,7 @@
 
 static struct omap_uart {
     u32 baud, clock_hz, data_bits, parity, stop_bits, fifo_size;
-    struct dt_irq irq;
+    unsigned int irq;
     char __iomem *regs;
     struct irqaction irqaction;
     struct vuart_info vuart;
@@ -205,10 +205,10 @@ static void __init omap_uart_init_postirq(struct serial_port *port)
     uart->irqaction.name = "omap_uart";
     uart->irqaction.dev_id = port;
 
-    if ( setup_dt_irq(&uart->irq, &uart->irqaction) != 0 )
+    if ( setup_irq(uart->irq, &uart->irqaction) != 0 )
     {
         dprintk(XENLOG_ERR, "Failed to allocated omap_uart IRQ %d\n",
-                uart->irq.irq);
+                uart->irq);
         return;
     }
 
@@ -259,7 +259,7 @@ static int __init omap_uart_irq(struct serial_port *port)
 {
     struct omap_uart *uart = port->uart;
 
-    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
+    return ((uart->irq > 0) ? uart->irq : -1);
 }
 
 static const struct vuart_info *omap_vuart_info(struct serial_port *port)
@@ -317,12 +317,13 @@ static int __init omap_uart_init(struct dt_device_node *dev,
         return res;
     }
 
-    res = dt_device_get_irq(dev, 0, &uart->irq);
-    if ( res )
+    res = platform_get_irq(dev, 0);
+    if ( res < 0 )
     {
         printk("omap-uart: Unable to retrieve the IRQ\n");
-        return res;
+        return -EINVAL;
     }
+    uart->irq = res;
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 459e686..89bda94 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -32,7 +32,7 @@
 
 static struct pl011 {
     unsigned int baud, clock_hz, data_bits, parity, stop_bits;
-    struct dt_irq irq;
+    unsigned int irq;
     void __iomem *regs;
     /* UART with IRQ line: interrupt-driven I/O. */
     struct irqaction irqaction;
@@ -132,13 +132,13 @@ static void __init pl011_init_postirq(struct serial_port *port)
     struct pl011 *uart = port->uart;
     int rc;
 
-    if ( uart->irq.irq > 0 )
+    if ( uart->irq > 0 )
     {
         uart->irqaction.handler = pl011_interrupt;
         uart->irqaction.name    = "pl011";
         uart->irqaction.dev_id  = port;
-        if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
-            printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq.irq);
+        if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+            printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq);
     }
 
     /* Clear pending error interrupts */
@@ -186,7 +186,8 @@ static int pl011_getc(struct serial_port *port, char *pc)
 static int __init pl011_irq(struct serial_port *port)
 {
     struct pl011 *uart = port->uart;
-    return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
+
+    return ((uart->irq > 0) ? uart->irq : -1);
 }
 
 static const struct vuart_info *pl011_vuart(struct serial_port *port)
@@ -239,12 +240,13 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
         return res;
     }
 
-    res = dt_device_get_irq(dev, 0, &uart->irq);
-    if ( res )
+    res = platform_get_irq(dev, 0);
+    if ( res < 0 )
     {
         printk("pl011: Unable to retrieve the IRQ\n");
-        return res;
+        return -EINVAL;
     }
+    uart->irq = res;
 
     uart->regs = ioremap_nocache(addr, size);
     if ( !uart->regs )
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 93d2128..f9b9a9f 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -40,11 +40,6 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq);
 void init_IRQ(void);
 void init_secondary_IRQ(void);
 
-int request_dt_irq(const struct dt_irq *irq,
-                   void (*handler)(int, void *, struct cpu_user_regs *),
-                   const char *devname, void *dev_id);
-int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new);
-
 int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
                           const char *devname);
 int platform_get_irq(const struct dt_device_node *device,

-- 
Julien Grall

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ
  2014-04-22 12:58 ` [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ Julien Grall
@ 2014-04-22 13:36   ` Jan Beulich
  2014-04-23 14:07   ` Ian Campbell
  1 sibling, 0 replies; 33+ messages in thread
From: Jan Beulich @ 2014-04-22 13:36 UTC (permalink / raw)
  To: Julien Grall
  Cc: xen-devel, Keir Fraser, stefano.stabellini, ian.campbell, tim

>>> On 22.04.14 at 14:58, <julien.grall@linaro.org> wrote:
> On ARM, it may happen (eg ARM SMMU) to setup multiple handler for the same
> interrupt.
> 
> To be able to use multiple action, the driver has to explicitly call
> {setup,request}_irq with IRQF_SHARED as 2nd parameter.
> 
> The behavior stays the same on x86, e.g only one action is handled.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

For the non-ARM part:
Acked-by: Jan Beulich <jbeulich@suse.com>

> ---
>     Changes in v4:
>         - Go back to a single custom linked list. The double linked-list
>         doesn't fit the requirements (i.e browsing safely without look) and
>         the llist.h from Linux doesn't allow use to delete a node in the 
> middle
>         of the list.
> 
>     Changes in v3:
>         - Drop {setup,request}_irq_flags, the current function has been
>         extended on an earlier patch.
>         - Rename IRQ_SHARED into IRQF_SHARED
> 
>     Changes in v2:
>         - Explain design choice
>         - Introduce CONFIG_IRQ_HAS_MULTIPLE_ACTION
>         - Use list instead of custom pointer
>         - release_irq should not shutdown the IRQ at the beginning
>         - Add IRQ_SHARED flags
>         - Introduce request_irq_flags and setup_irq_flags
>         - Fix compilation on x86. Some code are creating the irqaction
>         via = { ... } so the "next" field should be moved
>         toward the end
> ---
>  xen/arch/arm/irq.c           |   79 +++++++++++++++++++++++++++++++++---------
>  xen/include/asm-arm/config.h |    2 ++
>  xen/include/xen/irq.h        |    4 +++
>  3 files changed, 68 insertions(+), 17 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 1bc960d..d05c0be 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -155,7 +155,6 @@ int request_irq(unsigned int irq, unsigned int irqflags,
>  void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
>  {
>      struct irq_desc *desc = irq_to_desc(irq);
> -    struct irqaction *action = desc->action;
>  
>      /* TODO: perfc_incr(irqs); */
>  
> @@ -166,7 +165,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, 
> int is_fiq)
>      spin_lock(&desc->lock);
>      desc->handler->ack(desc);
>  
> -    if ( action == NULL )
> +    if ( !desc->action )
>      {
>          printk("Unknown %s %#3.3x\n",
>                 is_fiq ? "FIQ" : "IRQ", irq);
> @@ -198,12 +197,21 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int 
> irq, int is_fiq)
>  
>      desc->status |= IRQ_INPROGRESS;
>  
> -    action = desc->action;
>      while ( desc->status & IRQ_PENDING )
>      {
> +        struct irqaction *action;
> +
>          desc->status &= ~IRQ_PENDING;
> +        action = desc->action;
> +
>          spin_unlock_irq(&desc->lock);
> -        action->handler(irq, action->dev_id, regs);
> +
> +        do
> +        {
> +            action->handler(irq, action->dev_id, regs);
> +            action = action->next;
> +        } while ( action );
> +
>          spin_lock_irq(&desc->lock);
>      }
>  
> @@ -220,34 +228,71 @@ void release_irq(unsigned int irq, const void *dev_id)
>  {
>      struct irq_desc *desc;
>      unsigned long flags;
> -   struct irqaction *action;
> +    struct irqaction *action, **action_ptr;
>  
>      desc = irq_to_desc(irq);
>  
>      spin_lock_irqsave(&desc->lock,flags);
>  
> -    desc->handler->shutdown(desc);
> +    action_ptr = &desc->action;
> +    for ( ;; )
> +    {
> +        action = *action_ptr;
> +        if ( !action )
> +        {
> +            printk(XENLOG_WARNING "Trying to free already-free IRQ %u\n", 
> irq);
> +            spin_unlock_irqrestore(&desc->lock, flags);
> +            return;
> +        }
> +
> +        if ( action->dev_id == dev_id )
> +            break;
> +
> +        action_ptr = &action->next;
> +    }
> +
> +    /* Found it - remove it from the action list */
> +    *action_ptr = action->next;
>  
> -    action = desc->action;
> -    desc->action  = NULL;
> -    desc->status &= ~IRQ_GUEST;
> +    /* If this was the last action, shut down the IRQ */
> +    if ( !desc->action )
> +    {
> +        desc->handler->shutdown(desc);
> +        desc->status &= ~IRQ_GUEST;
> +    }
>  
>      spin_unlock_irqrestore(&desc->lock,flags);
>  
>      /* Wait to make sure it's not being used on another CPU */
>      do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
>  
> -    if ( action && action->free_on_release )
> +    if ( action->free_on_release )
>          xfree(action);
>  }
>  
> -static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
> +static int __setup_irq(struct irq_desc *desc, unsigned int irqflags,
> +                       struct irqaction *new)
>  {
> -    if ( desc->action != NULL )
> -        return -EBUSY;
> +    bool_t shared = !!(irqflags & IRQF_SHARED);
> +
> +    ASSERT(new != NULL);
> +
> +    /* Sanity checks:
> +     *  - if the IRQ is marked as shared
> +     *  - dev_id is not NULL when IRQF_SHARED is set
> +     */
> +    if ( desc->action != NULL && (!(desc->status & IRQF_SHARED) || !shared) )
> +        return -EINVAL;
> +    if ( shared && new->dev_id == NULL )
> +        return -EINVAL;
> +
> +    if ( shared )
> +        desc->status |= IRQF_SHARED;
>  
> -    desc->action  = new;
> -    dsb(sy);
> +    new->next = desc->action;
> +    dsb(ish);
> +    desc->action = new;
> +    dsb(ish);
>  
>      return 0;
>  }
> @@ -275,7 +320,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, 
> struct irqaction *new)
>  
>      disabled = (desc->action == NULL);
>  
> -    rc = __setup_irq(desc, new);
> +    rc = __setup_irq(desc, irqflags, new);
>      if ( rc )
>          goto err;
>  
> @@ -340,7 +385,7 @@ int route_dt_irq_to_guest(struct domain *d, const struct 
> dt_irq *irq,
>          goto out;
>      }
>  
> -    retval = __setup_irq(desc, action);
> +    retval = __setup_irq(desc, 0, action);
>      if ( retval )
>          goto out;
>  
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index ef291ff..1c3abcf 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -37,6 +37,8 @@
>  
>  #define CONFIG_VIDEO 1
>  
> +#define CONFIG_IRQ_HAS_MULTIPLE_ACTION 1
> +
>  #define OPT_CONSOLE_STR "dtuart"
>  
>  #ifdef MAX_PHYS_CPUS
> diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
> index f9a18d8..40c0f3f 100644
> --- a/xen/include/xen/irq.h
> +++ b/xen/include/xen/irq.h
> @@ -14,6 +14,9 @@ struct irqaction {
>      const char *name;
>      void *dev_id;
>      bool_t free_on_release;
> +#ifdef CONFIG_IRQ_HAS_MULTIPLE_ACTION
> +    struct irqaction *next;
> +#endif
>  };
>  
>  /*
> @@ -27,6 +30,7 @@ struct irqaction {
>  #define IRQ_MOVE_PENDING  (1u<<5) /* IRQ is migrating to another CPUs */
>  #define IRQ_PER_CPU       (1u<<6) /* IRQ is per CPU */
>  #define IRQ_GUEST_EOI_PENDING (1u<<7) /* IRQ was disabled, pending a guest 
> EOI */
> +#define IRQF_SHARED       (1<<8)  /* IRQ is shared */
>  
>  /* Special IRQ numbers. */
>  #define AUTO_ASSIGN_IRQ         (-1)
> -- 
> 1.7.10.4

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc
  2014-04-22 12:58 ` [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc Julien Grall
@ 2014-04-23 12:26   ` Julien Grall
  2014-04-23 14:04   ` Ian Campbell
  1 sibling, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-23 12:26 UTC (permalink / raw)
  To: ian.campbell; +Cc: xen-devel, Julien Grall, tim, stefano.stabellini

On 04/22/2014 01:58 PM, Julien Grall wrote:
> @@ -82,6 +83,12 @@ static int __cpuinit init_local_irq_data(void)
>          init_one_irq_desc(desc);
>          desc->irq = irq;
>          desc->action  = NULL;
> +
> +        /* PPIs are include in local_irqs, we have to copy the IRQ type from
> +         * CPU0 otherwise we may miss the type if the IRQ type has been
> +         * set early.
> +         */
> +        desc->arch.type = per_cpu(local_irq_desc, 0)[irq].arch.type;

I forgot to introduce a boot_cpu macro (as asked by Ian on v2).

I will write a follow-up to remove per_cpu(..., 0) in few places if I
don't need to resent this series.


-- 
Julien Grall

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc
  2014-04-22 12:58 ` [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc Julien Grall
  2014-04-23 12:26   ` Julien Grall
@ 2014-04-23 14:04   ` Ian Campbell
  2014-05-02  8:49     ` Julien Grall
  1 sibling, 1 reply; 33+ messages in thread
From: Ian Campbell @ 2014-04-23 14:04 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, tim, stefano.stabellini

On Tue, 2014-04-22 at 13:58 +0100, Julien Grall wrote:
> For now, ARM uses different IRQ functions to setup an interrupt handler. This
> is a bit annoying for common driver because we have to add idefery when
> an IRQ is setup (see ns16550_init_postirq for an example).
> 
> To avoid to completely fork the IRQ management code, we can introduce a field
> to store the IRQ type (e.g level/edge ...).
> 
> This patch also adds platform_get_irq which will retrieve the IRQ from the
> device tree and setup correctly the IRQ type.
> 
> In order to use this solution, we have to move init_IRQ earlier for the boot
> CPU. It's fine because the code only depends on percpu.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

Mostly just grammar nits.

> @@ -82,6 +83,12 @@ static int __cpuinit init_local_irq_data(void)
>          init_one_irq_desc(desc);
>          desc->irq = irq;
>          desc->action  = NULL;
> +
> +        /* PPIs are include in local_irqs, we have to copy the IRQ type from

"included"

> +         * CPU0 otherwise we may miss the type if the IRQ type has been
> +         * set early.

what does "set early" mean, how early is early? Initialised before
secondary CPUs were brought up perhaps?

        /* PPIs are included in local_irqs, we copy the IRQ type from
         * CPU0 when bringing up secondary cpus in order to pick up any
         * configuration done before this CPU came up. For interrupts
         * configured after this point this is done in XXX().

Why is the copying not conditional on CPU!=0?

> +    /* Setup the IRQ type */
> +    if ( irq < NR_LOCAL_IRQS )
> +    {
> +        unsigned int cpu;
> +        /* For PPIs, we need to set IRQ type on every online CPUs */

CPU should be singular here.

> +        for_each_cpu( cpu, &cpu_online_map )
> +        {
> +            desc = &per_cpu(local_irq_desc, cpu)[irq];
> +            res = irq_set_type(desc, type);
> +            if ( res )
> +            {
> +                /*
> +                 * For PPIs the IRQ type is the same on every CPU. It
> +                 * should not fail to other CPU than CPU0

"It should not fail on other CPUs than CPU0" (or "cannot fail" would be
stronger, and an assert is pretty strong).

Ian.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ
  2014-04-22 12:58 ` [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ Julien Grall
  2014-04-22 13:36   ` Jan Beulich
@ 2014-04-23 14:07   ` Ian Campbell
  2014-04-23 14:56     ` Julien Grall
  1 sibling, 1 reply; 33+ messages in thread
From: Ian Campbell @ 2014-04-23 14:07 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Keir Fraser, tim, Jan Beulich, stefano.stabellini

On Tue, 2014-04-22 at 13:58 +0100, Julien Grall wrote:
> On ARM, it may happen (eg ARM SMMU) to setup multiple handler for the same
> interrupt.
> 
> To be able to use multiple action, the driver has to explicitly call
> {setup,request}_irq with IRQF_SHARED as 2nd parameter.
> 
> The behavior stays the same on x86, e.g only one action is handled.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Jan Beulich <jbeulich@suse.com>
> 
> ---
>     Changes in v4:
>         - Go back to a single custom linked list. The double linked-list
>         doesn't fit the requirements (i.e browsing safely without look) and
>         the llist.h from Linux doesn't allow use to delete a node in the middle
>         of the list.

Is this variant any safer?

> +        do
> +        {
> +            action->handler(irq, action->dev_id, regs);
> +            action = action->next;
> +        } while ( action );

What happens if action is freed and recycled in the midst of this loop?

Ian.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ
  2014-04-23 14:07   ` Ian Campbell
@ 2014-04-23 14:56     ` Julien Grall
  2014-04-23 15:16       ` Ian Campbell
  0 siblings, 1 reply; 33+ messages in thread
From: Julien Grall @ 2014-04-23 14:56 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Keir Fraser, tim, Jan Beulich, stefano.stabellini

On 04/23/2014 03:07 PM, Ian Campbell wrote:
> On Tue, 2014-04-22 at 13:58 +0100, Julien Grall wrote:
>> On ARM, it may happen (eg ARM SMMU) to setup multiple handler for the same
>> interrupt.
>>
>> To be able to use multiple action, the driver has to explicitly call
>> {setup,request}_irq with IRQF_SHARED as 2nd parameter.
>>
>> The behavior stays the same on x86, e.g only one action is handled.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> Cc: Keir Fraser <keir@xen.org>
>> Cc: Jan Beulich <jbeulich@suse.com>
>>
>> ---
>>     Changes in v4:
>>         - Go back to a single custom linked list. The double linked-list
>>         doesn't fit the requirements (i.e browsing safely without look) and
>>         the llist.h from Linux doesn't allow use to delete a node in the middle
>>         of the list.
> 
> Is this variant any safer?

Yes, everything is protected by a desc->lock, except in do_IRQ (see
below why).

>> +        do
>> +        {
>> +            action->handler(irq, action->dev_id, regs);
>> +            action = action->next;
>> +        } while ( action );
> 
> What happens if action is freed and recycled in the midst of this loop?

Nothing, the action won't be free until IRQ_INPROGRESS is set (see do ..
while at the end of release_irq).

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ
  2014-04-23 14:56     ` Julien Grall
@ 2014-04-23 15:16       ` Ian Campbell
  2014-04-23 15:28         ` Julien Grall
  2014-04-23 15:28         ` Julien Grall
  0 siblings, 2 replies; 33+ messages in thread
From: Ian Campbell @ 2014-04-23 15:16 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Keir Fraser, tim, Jan Beulich, stefano.stabellini

On Wed, 2014-04-23 at 15:56 +0100, Julien Grall wrote:
> >> +        do
> >> +        {
> >> +            action->handler(irq, action->dev_id, regs);
> >> +            action = action->next;
> >> +        } while ( action );
> > 
> > What happens if action is freed and recycled in the midst of this loop?
> 
> Nothing, the action won't be free until IRQ_INPROGRESS is set (see do ..
> while at the end of release_irq).

BY that logic wasn't the version which used the list macros also safe
then?

Ian.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ
  2014-04-23 15:16       ` Ian Campbell
@ 2014-04-23 15:28         ` Julien Grall
  2014-04-23 15:28         ` Julien Grall
  1 sibling, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-23 15:28 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Keir Fraser, tim, Jan Beulich, stefano.stabellini

On 04/23/2014 04:16 PM, Ian Campbell wrote:
> On Wed, 2014-04-23 at 15:56 +0100, Julien Grall wrote:
>>>> +        do
>>>> +        {
>>>> +            action->handler(irq, action->dev_id, regs);
>>>> +            action = action->next;
>>>> +        } while ( action );
>>>
>>> What happens if action is freed and recycled in the midst of this loop?
>>
>> Nothing, the action won't be free until IRQ_INPROGRESS is set (see do ..
>> while at the end of release_irq).
> 
> BY that logic wasn't the version which used the list macros also safe
> then?

No because we had to modify two pointers (next and prev). Here as we
modify only one pointer, do_IRQ may or may not call action for a last time.

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ
  2014-04-23 15:16       ` Ian Campbell
  2014-04-23 15:28         ` Julien Grall
@ 2014-04-23 15:28         ` Julien Grall
  1 sibling, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-04-23 15:28 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, Keir Fraser, tim, Jan Beulich, stefano.stabellini

On 04/23/2014 04:16 PM, Ian Campbell wrote:
> On Wed, 2014-04-23 at 15:56 +0100, Julien Grall wrote:
>>>> +        do
>>>> +        {
>>>> +            action->handler(irq, action->dev_id, regs);
>>>> +            action = action->next;
>>>> +        } while ( action );
>>>
>>> What happens if action is freed and recycled in the midst of this loop?
>>
>> Nothing, the action won't be free until IRQ_INPROGRESS is set (see do ..
>> while at the end of release_irq).
> 
> BY that logic wasn't the version which used the list macros also safe
> then?

No because we had to modify two pointers (next and prev). Here as we
modify only one pointer, do_IRQ may or may not call action for a last time.

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc
  2014-04-23 14:04   ` Ian Campbell
@ 2014-05-02  8:49     ` Julien Grall
  2014-05-02  8:57       ` Ian Campbell
  0 siblings, 1 reply; 33+ messages in thread
From: Julien Grall @ 2014-05-02  8:49 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, tim, stefano.stabellini

Hi Ian,

Sorry for the delay, for some unknown reason I skipped this mail.

On 23/04/14 15:04, Ian Campbell wrote:
>> +         * CPU0 otherwise we may miss the type if the IRQ type has been
>> +         * set early.
>
> what does "set early" mean, how early is early? Initialised before
> secondary CPUs were brought up perhaps?

Yes. I will replace "set early" by "initialised before secondary CPUs 
were brought up"

>
>          /* PPIs are included in local_irqs, we copy the IRQ type from
>           * CPU0 when bringing up secondary cpus in order to pick up any
>           * configuration done before this CPU came up. For interrupts
>           * configured after this point this is done in XXX().

I guess you suggest to replace the previous comment by this one, right?

> Why is the copying not conditional on CPU!=0?

As the CPU were not online (therefore its per_cpu regions is not there) 
at the time we setup the IRQ we have to unconditionally set the IRQ type.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 00/18] xen/arm: Interrupt management reworking
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (17 preceding siblings ...)
  2014-04-22 12:58 ` [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ Julien Grall
@ 2014-05-02  8:52 ` Julien Grall
  2014-05-02 12:51 ` Ian Campbell
  19 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-05-02  8:52 UTC (permalink / raw)
  To: ian.campbell; +Cc: xen-devel, tim, stefano.stabellini

Ian,

Patch #1-13 are all acked, can you apply them to staging?

It will avoid me to send the whole series again.

Thanks !

On 22/04/14 13:58, Julien Grall wrote:
> Hello,
>
> This is the fourth version of this serie to rework interrupt management for
> ARM.
>
> Major changes in v4:
>      - Use a custom single linked list for handling multiple action
>      - Slightly rework patch #9 (was acked by Ian)
>      - Remove dependency on Stefano's series for Patch #10
>      - Change platform_get_irq error code to -1 (impact patch #14 acked by Ian)
>
> For all changes see in each patch.
>
> This series is a dependency for the ARM SMMU drivers.
>
> A working tree can be found here:
>      git://xenbits.xen.org/people/julieng/xen-unstable.git branch interrupt-mgmt-v4
>
> Sincerely yours,
>
> Julien Grall (18):
>    xen/arm: timer: replace timer_dt_irq by timer_get_irq
>    xen/arm: IRQ: Use default irq callback from common code for
>      no_irq_type
>    xen/arm: IRQ: Rename irq_cfg into arch_irq_desc
>    xen/arm: IRQ: move gic {,un}lock in gic_set_irq_properties
>    xen/arm: IRQ: drop irq parameter in __setup_irq
>    xen/arm: IRQ: remove __init from setup_dt_irq, request_dt_irq and
>      release_irq
>    xen/arm: IRQ: Rework gic_route_irq_to_guest function
>    xen/arm: IRQ: Move IRQ management from gic.c to irq.c
>    xen/arm: IRQ Introduce irq_get_domain
>    xen/arm: IRQ: Require desc.lock be held by callers of
>      hw_irq_controller callbacks
>    xen/arm: IRQ: Defer routing IRQ to Xen until setup_irq() call
>    xen/arm: IRQ: Do not allow IRQ to be shared between domains and XEN
>    xen/serial: remove serial_dt_irq
>    xen/arm: IRQ: Store IRQ type in arch_irq_desc
>    xen/arm: IRQ: Replace {request,setup}_dt_irq by {request,setup}_irq
>    xen: IRQ: Add dev_id parameter to release_irq
>    xen/arm: IRQ: extend {request,setup}_irq to take an irqflags in
>      parameter
>    xen/arm: IRQ: Handle multiple action per IRQ
>
>   xen/arch/arm/domain_build.c              |   32 +--
>   xen/arch/arm/gic.c                       |  230 +++++----------------
>   xen/arch/arm/irq.c                       |  321 ++++++++++++++++++++++++++++--
>   xen/arch/arm/platforms/xgene-storm.c     |    2 +-
>   xen/arch/arm/setup.c                     |    5 +-
>   xen/arch/arm/smpboot.c                   |    2 -
>   xen/arch/arm/time.c                      |   43 ++--
>   xen/arch/arm/vgic.c                      |   10 +
>   xen/arch/arm/vtimer.c                    |    4 +-
>   xen/arch/x86/hpet.c                      |    2 +-
>   xen/arch/x86/i8259.c                     |    2 +-
>   xen/arch/x86/irq.c                       |   11 +-
>   xen/arch/x86/time.c                      |    2 +-
>   xen/drivers/char/exynos4210-uart.c       |   23 +--
>   xen/drivers/char/ns16550.c               |   31 +--
>   xen/drivers/char/omap-uart.c             |   23 +--
>   xen/drivers/char/pl011.c                 |   24 +--
>   xen/drivers/char/serial.c                |    9 -
>   xen/drivers/passthrough/amd/iommu_init.c |    2 +-
>   xen/drivers/passthrough/vtd/iommu.c      |    2 +-
>   xen/include/asm-arm/config.h             |    2 +
>   xen/include/asm-arm/gic.h                |   16 +-
>   xen/include/asm-arm/irq.h                |   12 +-
>   xen/include/asm-arm/time.h               |    7 +-
>   xen/include/xen/irq.h                    |   11 +-
>   xen/include/xen/serial.h                 |    5 -
>   26 files changed, 471 insertions(+), 362 deletions(-)
>

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc
  2014-05-02  8:49     ` Julien Grall
@ 2014-05-02  8:57       ` Ian Campbell
  2014-05-02  9:00         ` Julien Grall
  0 siblings, 1 reply; 33+ messages in thread
From: Ian Campbell @ 2014-05-02  8:57 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, tim, stefano.stabellini

On Fri, 2014-05-02 at 09:49 +0100, Julien Grall wrote:
> Hi Ian,
> 
> Sorry for the delay, for some unknown reason I skipped this mail.
> 
> On 23/04/14 15:04, Ian Campbell wrote:
> >> +         * CPU0 otherwise we may miss the type if the IRQ type has been
> >> +         * set early.
> >
> > what does "set early" mean, how early is early? Initialised before
> > secondary CPUs were brought up perhaps?
> 
> Yes. I will replace "set early" by "initialised before secondary CPUs 
> were brought up"
> 
> >
> >          /* PPIs are included in local_irqs, we copy the IRQ type from
> >           * CPU0 when bringing up secondary cpus in order to pick up any
> >           * configuration done before this CPU came up. For interrupts
> >           * configured after this point this is done in XXX().
> 
> I guess you suggest to replace the previous comment by this one, right?

If it is factually accurate, yes.

> > Why is the copying not conditional on CPU!=0?
> 
> As the CPU were not online (therefore its per_cpu regions is not there) 
> at the time we setup the IRQ we have to unconditionally set the IRQ type.

But CPU0 must have been online when we setup the IRQ.

IOW if CPU==0 then isn't 
        desc->arch.type = per_cpu(local_irq_desc, 0)[irq].arch.type;
equivalent to
        desc->arch.type = desc->arch.type
        
?

Ian.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc
  2014-05-02  8:57       ` Ian Campbell
@ 2014-05-02  9:00         ` Julien Grall
  0 siblings, 0 replies; 33+ messages in thread
From: Julien Grall @ 2014-05-02  9:00 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel, tim, stefano.stabellini



On 02/05/14 09:57, Ian Campbell wrote:
  > But CPU0 must have been online when we setup the IRQ.
>
> IOW if CPU==0 then isn't
>          desc->arch.type = per_cpu(local_irq_desc, 0)[irq].arch.type;
> equivalent to
>          desc->arch.type = desc->arch.type
>
> ?

Hmmmm yes, I forgot this case. I will update the patch in the next version.

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 00/18] xen/arm: Interrupt management reworking
  2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
                   ` (18 preceding siblings ...)
  2014-05-02  8:52 ` [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
@ 2014-05-02 12:51 ` Ian Campbell
  19 siblings, 0 replies; 33+ messages in thread
From: Ian Campbell @ 2014-05-02 12:51 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, tim, stefano.stabellini

On Tue, 2014-04-22 at 13:58 +0100, Julien Grall wrote:
> Julien Grall (18):
>   xen/arm: timer: replace timer_dt_irq by timer_get_irq
>   xen/arm: IRQ: Use default irq callback from common code for
>     no_irq_type
>   xen/arm: IRQ: Rename irq_cfg into arch_irq_desc
>   xen/arm: IRQ: move gic {,un}lock in gic_set_irq_properties
>   xen/arm: IRQ: drop irq parameter in __setup_irq
>   xen/arm: IRQ: remove __init from setup_dt_irq, request_dt_irq and
>     release_irq
>   xen/arm: IRQ: Rework gic_route_irq_to_guest function
>   xen/arm: IRQ: Move IRQ management from gic.c to irq.c
>   xen/arm: IRQ Introduce irq_get_domain
>   xen/arm: IRQ: Require desc.lock be held by callers of
>     hw_irq_controller callbacks
>   xen/arm: IRQ: Defer routing IRQ to Xen until setup_irq() call
>   xen/arm: IRQ: Do not allow IRQ to be shared between domains and XEN
>   xen/serial: remove serial_dt_irq

Applied up to here.

>   xen/arm: IRQ: Store IRQ type in arch_irq_desc
>   xen/arm: IRQ: Replace {request,setup}_dt_irq by {request,setup}_irq
>   xen: IRQ: Add dev_id parameter to release_irq
>   xen/arm: IRQ: extend {request,setup}_irq to take an irqflags in
>     parameter
>   xen/arm: IRQ: Handle multiple action per IRQ

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2014-05-02 12:52 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-22 12:58 [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
2014-04-22 12:58 ` [PATCH v4 01/18] xen/arm: timer: replace timer_dt_irq by timer_get_irq Julien Grall
2014-04-22 12:58 ` [PATCH v4 02/18] xen/arm: IRQ: Use default irq callback from common code for no_irq_type Julien Grall
2014-04-22 12:58 ` [PATCH v4 03/18] xen/arm: IRQ: Rename irq_cfg into arch_irq_desc Julien Grall
2014-04-22 12:58 ` [PATCH v4 04/18] xen/arm: IRQ: move gic {, un}lock in gic_set_irq_properties Julien Grall
2014-04-22 12:58 ` [PATCH v4 05/18] xen/arm: IRQ: drop irq parameter in __setup_irq Julien Grall
2014-04-22 12:58 ` [PATCH v4 06/18] xen/arm: IRQ: remove __init from setup_dt_irq, request_dt_irq and release_irq Julien Grall
2014-04-22 12:58 ` [PATCH v4 07/18] xen/arm: IRQ: Rework gic_route_irq_to_guest function Julien Grall
2014-04-22 12:58 ` [PATCH v4 08/18] xen/arm: IRQ: Move IRQ management from gic.c to irq.c Julien Grall
2014-04-22 12:58 ` [PATCH v4 09/18] xen/arm: IRQ Introduce irq_get_domain Julien Grall
2014-04-22 12:58 ` [PATCH v4 10/18] xen/arm: IRQ: Require desc.lock be held by callers of hw_irq_controller callbacks Julien Grall
2014-04-22 12:58 ` [PATCH v4 11/18] xen/arm: IRQ: Defer routing IRQ to Xen until setup_irq() call Julien Grall
2014-04-22 12:58 ` [PATCH v4 12/18] xen/arm: IRQ: Do not allow IRQ to be shared between domains and XEN Julien Grall
2014-04-22 12:58 ` [PATCH v4 13/18] xen/serial: remove serial_dt_irq Julien Grall
2014-04-22 12:58 ` [PATCH v4 14/18] xen/arm: IRQ: Store IRQ type in arch_irq_desc Julien Grall
2014-04-23 12:26   ` Julien Grall
2014-04-23 14:04   ` Ian Campbell
2014-05-02  8:49     ` Julien Grall
2014-05-02  8:57       ` Ian Campbell
2014-05-02  9:00         ` Julien Grall
2014-04-22 12:58 ` [PATCH v4 15/18] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq Julien Grall
2014-04-22 13:26   ` Julien Grall
2014-04-22 12:58 ` [PATCH v4 16/18] xen: IRQ: Add dev_id parameter to release_irq Julien Grall
2014-04-22 12:58 ` [PATCH v4 17/18] xen/arm: IRQ: extend {request, setup}_irq to take an irqflags in parameter Julien Grall
2014-04-22 12:58 ` [PATCH v4 18/18] xen/arm: IRQ: Handle multiple action per IRQ Julien Grall
2014-04-22 13:36   ` Jan Beulich
2014-04-23 14:07   ` Ian Campbell
2014-04-23 14:56     ` Julien Grall
2014-04-23 15:16       ` Ian Campbell
2014-04-23 15:28         ` Julien Grall
2014-04-23 15:28         ` Julien Grall
2014-05-02  8:52 ` [PATCH v4 00/18] xen/arm: Interrupt management reworking Julien Grall
2014-05-02 12:51 ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.