[Xen-devel] [PATCH v2 16/16] xen/arm: IRQ: Handle multiple action per IRQ

[Julien Grall <julien.grall@xxxxxxxxxx>]

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_flags (newly created) with IRQ_SHARED as 3rd parameter.

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

Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
Cc: Keir Fraser <keir@xxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>

---
    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           |  106 +++++++++++++++++++++++++++++++-----------
 xen/common/irq.c             |    3 ++
 xen/include/asm-arm/config.h |    2 +
 xen/include/asm-arm/irq.h    |    7 +++
 xen/include/xen/irq.h        |    8 ++++
 5 files changed, 99 insertions(+), 27 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index ef67279..556c2e1 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -68,7 +68,6 @@ static int __init init_irq_data(void)
         struct irq_desc *desc = irq_to_desc(irq);
         init_one_irq_desc(desc);
         desc->irq = irq;
-        desc->action  = NULL;
     }
 
     return 0;
@@ -82,7 +81,6 @@ static int __cpuinit init_local_irq_data(void)
         struct irq_desc *desc = irq_to_desc(irq);
         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
@@ -107,16 +105,21 @@ void __cpuinit init_secondary_IRQ(void)
 
 static inline struct domain *irq_get_domain(struct irq_desc *desc)
 {
+    struct irqaction *action;
+
     ASSERT(spin_is_locked(&desc->lock));
     ASSERT(desc->status & IRQ_GUEST);
-    ASSERT(desc->action != NULL);
+    ASSERT(!list_empty(&desc->action));
+
+    action = list_entry(desc->action.next, struct irqaction, next);
 
-    return desc->action->dev_id;
+    return action->dev_id;
 }
 
-int request_irq(unsigned int irq,
-                void (*handler)(int, void *, struct cpu_user_regs *),
-                const char *devname, void *dev_id)
+int request_irq_flags(unsigned int irq,
+                      void (*handler)(int, void *, struct cpu_user_regs *),
+                      unsigned int irqflags,
+                      const char *devname, void *dev_id)
 {
     struct irqaction *action;
     int retval;
@@ -141,18 +144,24 @@ int request_irq(unsigned int irq,
     action->dev_id = dev_id;
     action->free_on_release = 1;
 
-    retval = setup_irq(irq, action);
+    retval = setup_irq_flags(irq, action, irqflags);
     if ( retval )
         xfree(action);
 
     return retval;
 }
 
+int request_irq(unsigned int irq,
+                void (*handler)(int, void *, struct cpu_user_regs *),
+                const char *devname, void *dev_id)
+{
+    return request_irq_flags(irq, handler, 0, devname, dev_id);
+}
+
 /* Dispatch an interrupt */
 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); */
 
@@ -163,7 +172,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 ( list_empty(&desc->action) )
     {
         printk("Unknown %s %#3.3x\n",
                is_fiq ? "FIQ" : "IRQ", irq);
@@ -195,12 +204,14 @@ 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, *next;
+
         desc->status &= ~IRQ_PENDING;
         spin_unlock_irq(&desc->lock);
-        action->handler(irq, action->dev_id, regs);
+        list_for_each_entry_safe(action, next, &desc->action, next)
+            action->handler(irq, action->dev_id, regs);
         spin_lock_irq(&desc->lock);
     }
 
@@ -217,39 +228,75 @@ void release_irq(unsigned int irq, const void *dev_id)
 {
     struct irq_desc *desc;
     unsigned long flags;
-   struct irqaction *action;
+    struct irqaction *action;
+    bool_t found = 0;
 
     desc = irq_to_desc(irq);
 
     spin_lock_irqsave(&desc->lock,flags);
 
-    desc->handler->shutdown(desc);
+    list_for_each_entry(action, &desc->action, next)
+    {
+        if ( action->dev_id == dev_id )
+        {
+            found  = 1;
+            break;
+         }
+    }
 
-    action = desc->action;
-    desc->action  = NULL;
-    desc->status &= ~IRQ_GUEST;
+    if ( !found )
+    {
+        printk(XENLOG_WARNING "Trying to free already-free IRQ %u\n", irq);
+        return;
+    }
+
+    /* Found it - remove it from the action list */
+    list_del_init(&action->next);
+
+    /* If this was the last action, shut down the IRQ */
+    if ( list_empty(&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, struct irqaction *new,
+                       unsigned int irqflags)
 {
-    if ( desc->action != NULL )
-        return -EBUSY;
+    bool_t shared = !!(irqflags & IRQ_SHARED);
+
+    ASSERT(new != NULL);
 
-    desc->action  = new;
+    /* Sanity checks:
+     *  - if the IRQ is marked as shared
+     *  - dev_id is not NULL when IRQ_SHARED is set
+     */
+    if ( !list_empty(&desc->action) && (!(desc->status & IRQ_SHARED) || 
!shared) )
+        return -EINVAL;
+    if ( shared && new->dev_id == NULL )
+        return -EINVAL;
+
+    if ( shared )
+        desc->status |= IRQ_SHARED;
+
+    INIT_LIST_HEAD(&new->next);
+    list_add_tail(&new->next, &desc->action);
     dsb(sy);
 
     return 0;
 }
 
-int setup_irq(unsigned int irq, struct irqaction *new)
+int setup_irq_flags(unsigned int irq, struct irqaction *new,
+                    unsigned int irqflags)
 {
     int rc;
     unsigned long flags;
@@ -270,9 +317,9 @@ int setup_irq(unsigned int irq, struct irqaction *new)
         return -EBUSY;
     }
 
-    disabled = (desc->action == NULL);
+    disabled = list_empty(&desc->action);
 
-    rc = __setup_irq(desc, new);
+    rc = __setup_irq(desc, new, irqflags);
     if ( rc )
         goto err;
 
@@ -297,6 +344,11 @@ err:
     return rc;
 }
 
+int setup_irq(unsigned int irq, struct irqaction *new)
+{
+    return setup_irq_flags(irq, new, 0);
+}
+
 int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
                           const char * devname)
 {
@@ -320,7 +372,7 @@ int route_dt_irq_to_guest(struct domain *d, const struct 
dt_irq *irq,
      *  - Otherwise -> For now, don't allow the IRQ to be shared between
      *  Xen and domains.
      */
-    if ( desc->action != NULL )
+    if ( !list_empty(&desc->action) )
     {
         struct domain *ad = irq_get_domain(desc);
 
@@ -337,7 +389,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, action, 0);
     if ( retval )
     {
         xfree(action);
diff --git a/xen/common/irq.c b/xen/common/irq.c
index 3e55dfa..688e48a 100644
--- a/xen/common/irq.c
+++ b/xen/common/irq.c
@@ -17,6 +17,9 @@ int init_one_irq_desc(struct irq_desc *desc)
     spin_lock_init(&desc->lock);
     cpumask_setall(desc->affinity);
     INIT_LIST_HEAD(&desc->rl_link);
+#ifdef CONFIG_IRQ_HAS_MULTIPLE_ACTION
+    INIT_LIST_HEAD(&desc->action);
+#endif
 
     err = arch_init_one_irq_desc(desc);
     if ( err )
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 5b7b1a8..079e8b9 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/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index ad2cc18..80dd452 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -45,6 +45,13 @@ int route_dt_irq_to_guest(struct domain *d, const struct 
dt_irq *irq,
 unsigned int platform_get_irq(const struct dt_device_node *device,
                               int index);
 
+int request_irq_flags(unsigned int irq,
+                      void (*handler)(int, void *, struct cpu_user_regs *),
+                      unsigned int irqflags,
+                      const char * devname, void *dev_id);
+int setup_irq_flags(unsigned int irq, struct irqaction *new,
+                    unsigned int irqflags);
+
 #endif /* _ASM_HW_IRQ_H */
 /*
  * Local variables:
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index 1f8bdb3..49a805b 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 list_head 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 IRQ_SHARED        (1<<8)  /* IRQ is shared */
 
 /* Special IRQ numbers. */
 #define AUTO_ASSIGN_IRQ         (-1)
@@ -68,7 +72,11 @@ typedef struct irq_desc {
     unsigned int status;        /* IRQ status */
     hw_irq_controller *handler;
     struct msi_desc   *msi_desc;
+#ifdef CONFIG_IRQ_HAS_MULTIPLE_ACTION
+    struct list_head action;    /* IRQ action list */
+#else
     struct irqaction *action;   /* IRQ action list */
+#endif
     int irq;
     spinlock_t lock;
     struct arch_irq_desc arch;
-- 
1.7.10.4


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