| Message ID | 1396968247-8768-19-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
>>> On 08.04.14 at 16:44, <julien.grall@linaro.org> wrote: > --- 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 Considering that this is a doubly linked list, "next" as the name is sort of odd. Jan
On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote: > 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); You aren't removing entries from within the loop so I don't think you need the _safe variant. > spin_lock_irq(&desc->lock); > } > > @@ -217,33 +220,69 @@ 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; Extra space before =. I actually think a goto found would actually be clearer here than the flag. for each if (got it) goto found printk(not found) return found: /* Found it. */ > + /* Sanity checks: > + * - if the IRQ is marked as shared > + * - dev_id is not NULL when IRQF_SHARED is set > + */ > + if ( !list_empty(&desc->action) && > + (!(desc->status & IRQF_SHARED) || !shared) ) > + return -EINVAL; Did you mean EBUSY? > @@ -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 */ This was never actually a list I think, and the comment is certainly wrong now. Ian.
Hi Jan, On 04/08/2014 03:59 PM, Jan Beulich wrote: >>>> On 08.04.14 at 16:44, <julien.grall@linaro.org> wrote: >> --- 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 > > Considering that this is a doubly linked list, "next" as the name is sort > of odd. I will rename into list. Regards,
On 04/16/2014 04:54 PM, Ian Campbell wrote: > On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote: >> 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); > > You aren't removing entries from within the loop so I don't think you > need the _safe variant. As we release the desc->lock here, it might be possible to have the list changed under the CPU feet by release_irq. With the double-linked list, how do we make sure that it won't happen? >> spin_lock_irq(&desc->lock); >> } >> >> @@ -217,33 +220,69 @@ 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; > > Extra space before =. > > I actually think a goto found would actually be clearer here than the > flag. I'm not a big fan of goto. Anyway, I will use it here if you think it's clearer. > for each > if (got it) > goto found > > printk(not found) > return > > found: > /* Found it. */ >> + /* Sanity checks: >> + * - if the IRQ is marked as shared >> + * - dev_id is not NULL when IRQF_SHARED is set >> + */ >> + if ( !list_empty(&desc->action) && >> + (!(desc->status & IRQF_SHARED) || !shared) ) >> + return -EINVAL; > > Did you mean EBUSY? Right, EBUSY would be better here. >> @@ -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 */ > > This was never actually a list I think, and the comment is certainly > wrong now. I guess it was copied from Linux :). I will change the comment into "IRQ action" Regards,
On Wed, 2014-04-16 at 17:06 +0100, Julien Grall wrote: > On 04/16/2014 04:54 PM, Ian Campbell wrote: > > On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote: > >> 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); > > > > You aren't removing entries from within the loop so I don't think you > > need the _safe variant. > > As we release the desc->lock here, it might be possible to have the list > changed under the CPU feet by release_irq. > > With the double-linked list, how do we make sure that it won't happen? Normally by using a lock. I don't know if even list_for_each_entry_safe is safe against concurrent changes to the list from other threads, I think it only refers to safe to changing the list within the loop in this thread. > > I actually think a goto found would actually be clearer here than the > > flag. > > I'm not a big fan of goto. > > Anyway, I will use it here if you think it's clearer. It has it's places, I think this is one of them. Ian.
>>> On 16.04.14 at 18:17, <Ian.Campbell@citrix.com> wrote: > On Wed, 2014-04-16 at 17:06 +0100, Julien Grall wrote: >> On 04/16/2014 04:54 PM, Ian Campbell wrote: >> > On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote: >> >> 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); >> > >> > You aren't removing entries from within the loop so I don't think you >> > need the _safe variant. >> >> As we release the desc->lock here, it might be possible to have the list >> changed under the CPU feet by release_irq. >> >> With the double-linked list, how do we make sure that it won't happen? > > Normally by using a lock. I don't know if even list_for_each_entry_safe > is safe against concurrent changes to the list from other threads, I > think it only refers to safe to changing the list within the loop in > this thread. Indeed. Jan
On 04/16/2014 05:17 PM, Ian Campbell wrote: > On Wed, 2014-04-16 at 17:06 +0100, Julien Grall wrote: >> On 04/16/2014 04:54 PM, Ian Campbell wrote: >>> On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote: >>>> 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); >>> >>> You aren't removing entries from within the loop so I don't think you >>> need the _safe variant. >> >> As we release the desc->lock here, it might be possible to have the list >> changed under the CPU feet by release_irq. >> >> With the double-linked list, how do we make sure that it won't happen? > > Normally by using a lock. I don't know if even list_for_each_entry_safe > is safe against concurrent changes to the list from other threads, I > think it only refers to safe to changing the list within the loop in > this thread. > Hmmmm... I'm wondering if we can keep desc->lock held while calling the action handler and enable IRQ. For SPI, we are fine as the same SPI can't be fired twice. For PPI, it's bank so it's fine. Did I miss something?
>>> On 16.04.14 at 18:23, <julien.grall@linaro.org> wrote: > On 04/16/2014 05:17 PM, Ian Campbell wrote: >> On Wed, 2014-04-16 at 17:06 +0100, Julien Grall wrote: >>> On 04/16/2014 04:54 PM, Ian Campbell wrote: >>>> On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote: >>>>> 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); >>>> >>>> You aren't removing entries from within the loop so I don't think you >>>> need the _safe variant. >>> >>> As we release the desc->lock here, it might be possible to have the list >>> changed under the CPU feet by release_irq. >>> >>> With the double-linked list, how do we make sure that it won't happen? >> >> Normally by using a lock. I don't know if even list_for_each_entry_safe >> is safe against concurrent changes to the list from other threads, I >> think it only refers to safe to changing the list within the loop in >> this thread. >> > > Hmmmm... I'm wondering if we can keep desc->lock held while calling the > action handler and enable IRQ. That would set you up for problems with the handler wanting to manipulate its IRQ (which might imply locking desc). I'd suggest looking at how Linux deals with this (synchronize_irq() in particular). Jan
On 04/17/2014 08:07 AM, Jan Beulich wrote: >>>> On 16.04.14 at 18:23, <julien.grall@linaro.org> wrote: >> On 04/16/2014 05:17 PM, Ian Campbell wrote: >>> On Wed, 2014-04-16 at 17:06 +0100, Julien Grall wrote: >>>> On 04/16/2014 04:54 PM, Ian Campbell wrote: >>>>> On Tue, 2014-04-08 at 15:44 +0100, Julien Grall wrote: >>>>>> 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); >>>>> >>>>> You aren't removing entries from within the loop so I don't think you >>>>> need the _safe variant. >>>> >>>> As we release the desc->lock here, it might be possible to have the list >>>> changed under the CPU feet by release_irq. >>>> >>>> With the double-linked list, how do we make sure that it won't happen? >>> >>> Normally by using a lock. I don't know if even list_for_each_entry_safe >>> is safe against concurrent changes to the list from other threads, I >>> think it only refers to safe to changing the list within the loop in >>> this thread. >>> >> >> Hmmmm... I'm wondering if we can keep desc->lock held while calling the >> action handler and enable IRQ. > > That would set you up for problems with the handler wanting to > manipulate its IRQ (which might imply locking desc). I'd suggest > looking at how Linux deals with this (synchronize_irq() in particular). The release_irq code is borrowed from Linux. We already synchronize at the end. We have to delete the element in the critical section to avoid race with adding a new element. synchronizing in the critical section is not possible because do_IRQ will have to take the lock to clear the IRQ_INPROGRESS flag. In Linux case, they are safe because they are using a single linked list.
>>> On 17.04.14 at 12:36, <julien.grall@linaro.org> wrote: > In Linux case, they are safe because they are using a single linked list. And is there a strong reason for us to use a doubly linked one? Jan
On 04/17/2014 12:15 PM, Jan Beulich wrote: >>>> On 17.04.14 at 12:36, <julien.grall@linaro.org> wrote: >> In Linux case, they are safe because they are using a single linked list. > > And is there a strong reason for us to use a doubly linked one? My first version was single-linked list... Ian asked me to use generic list on V1 to avoid open code. I was looking to port llist.h from Linux but it seems a bit overkill for action list. I'm happy to move back to my first solution with a "next" field (see https://patches.linaro.org/23687/). Regards,
On Thu, 2014-04-17 at 12:22 +0100, Julien Grall wrote: > On 04/17/2014 12:15 PM, Jan Beulich wrote: > >>>> On 17.04.14 at 12:36, <julien.grall@linaro.org> wrote: > >> In Linux case, they are safe because they are using a single linked list. > > > > And is there a strong reason for us to use a doubly linked one? > > My first version was single-linked list... Ian asked me to use generic > list on V1 to avoid open code. I also suggested that you could import a single linked list implementation if you thought it would be better. > I was looking to port llist.h from Linux but it seems a bit overkill for > action list. I don't see why. I'm sure there are other potential uses of singly linked lists which people just used a double list because it was simpler at the time. Ian.
On 04/17/2014 12:36 PM, Ian Campbell wrote: > On Thu, 2014-04-17 at 12:22 +0100, Julien Grall wrote: >> On 04/17/2014 12:15 PM, Jan Beulich wrote: >>>>>> On 17.04.14 at 12:36, <julien.grall@linaro.org> wrote: >>>> In Linux case, they are safe because they are using a single linked list. >>> >>> And is there a strong reason for us to use a doubly linked one? >> >> My first version was single-linked list... Ian asked me to use generic >> list on V1 to avoid open code. > > I also suggested that you could import a single linked list > implementation if you thought it would be better. > >> I was looking to port llist.h from Linux but it seems a bit overkill for >> action list. > > I don't see why. I'm sure there are other potential uses of singly > linked lists which people just used a double list because it was simpler > at the time. They are using xchg and AFAIU it's not possible to delete an element anywhere in the list. See: http://lxr.free-electrons.com/source/include/linux/llist.h
On Thu, 2014-04-17 at 13:18 +0100, Julien Grall wrote: > On 04/17/2014 12:36 PM, Ian Campbell wrote: > > On Thu, 2014-04-17 at 12:22 +0100, Julien Grall wrote: > >> On 04/17/2014 12:15 PM, Jan Beulich wrote: > >>>>>> On 17.04.14 at 12:36, <julien.grall@linaro.org> wrote: > >>>> In Linux case, they are safe because they are using a single linked list. > >>> > >>> And is there a strong reason for us to use a doubly linked one? > >> > >> My first version was single-linked list... Ian asked me to use generic > >> list on V1 to avoid open code. > > > > I also suggested that you could import a single linked list > > implementation if you thought it would be better. > > > >> I was looking to port llist.h from Linux but it seems a bit overkill for > >> action list. > > > > I don't see why. I'm sure there are other potential uses of singly > > linked lists which people just used a double list because it was simpler > > at the time. > > > They are using xchg and AFAIU it's not possible to delete an element > anywhere in the list. See: > > http://lxr.free-electrons.com/source/include/linux/llist.h Hrm, I thought Linux had a standard singly link list available too, oh well. Another option would be to delete things using the rcu mechanisms perhaps. Ian.
On 04/17/2014 01:27 PM, Ian Campbell wrote: > On Thu, 2014-04-17 at 13:18 +0100, Julien Grall wrote: >> On 04/17/2014 12:36 PM, Ian Campbell wrote: >>> On Thu, 2014-04-17 at 12:22 +0100, Julien Grall wrote: >>>> On 04/17/2014 12:15 PM, Jan Beulich wrote: >>>>>>>> On 17.04.14 at 12:36, <julien.grall@linaro.org> wrote: >>>>>> In Linux case, they are safe because they are using a single linked list. >>>>> >>>>> And is there a strong reason for us to use a doubly linked one? >>>> >>>> My first version was single-linked list... Ian asked me to use generic >>>> list on V1 to avoid open code. >>> >>> I also suggested that you could import a single linked list >>> implementation if you thought it would be better. >>> >>>> I was looking to port llist.h from Linux but it seems a bit overkill for >>>> action list. >>> >>> I don't see why. I'm sure there are other potential uses of singly >>> linked lists which people just used a double list because it was simpler >>> at the time. >> >> >> They are using xchg and AFAIU it's not possible to delete an element >> anywhere in the list. See: >> >> http://lxr.free-electrons.com/source/include/linux/llist.h > > Hrm, I thought Linux had a standard singly link list available too, oh > well. > > Another option would be to delete things using the rcu mechanisms > perhaps. Ok. Unless if you strongly disagree, I will stick into the "next" field. Using rcu... doesn't seems the right things for an IRQ management code.
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c index 18217e8..31edfc8 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,11 +105,15 @@ 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, unsigned int irqflags, @@ -152,7 +154,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); */ @@ -163,7 +164,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 +196,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,33 +220,69 @@ 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; + } + } + + if ( !found ) + { + printk(XENLOG_WARNING "Trying to free already-free IRQ %u\n", irq); + return; + } - action = desc->action; - desc->action = NULL; - desc->status &= ~IRQ_GUEST; + /* 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, 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 ( !list_empty(&desc->action) && + (!(desc->status & IRQF_SHARED) || !shared) ) + return -EINVAL; + if ( shared && new->dev_id == NULL ) + return -EINVAL; + + if ( shared ) + desc->status |= IRQF_SHARED; - desc->action = new; + INIT_LIST_HEAD(&new->next); + list_add_tail(&new->next, &desc->action); dsb(sy); return 0; @@ -270,9 +309,9 @@ int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new) return -EBUSY; } - disabled = (desc->action == NULL); + disabled = list_empty(&desc->action); - rc = __setup_irq(desc, new); + rc = __setup_irq(desc, irqflags, new); if ( rc ) goto err; @@ -320,7 +359,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 +376,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 ) { 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/xen/irq.h b/xen/include/xen/irq.h index f9a18d8..c42b022 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 IRQF_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;
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 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 | 83 +++++++++++++++++++++++++++++++----------- xen/common/irq.c | 3 ++ xen/include/asm-arm/config.h | 2 + xen/include/xen/irq.h | 8 ++++ 4 files changed, 74 insertions(+), 22 deletions(-)