irqdomain: Fix mapping-creation race
cover irqdomain: Fix mapping-creation race
From: Johan Hovold <[email protected]>
Here are backports of the irqdomain fixes that failed to apply to 5.15.
Johan
Johan Hovold (2):
irqdomain: Refactor __irq_domain_alloc_irqs()
irqdomain: Fix mapping-creation race
kernel/irq/irqdomain.c | 152 +++++++++++++++++++++++++---------------- 1 file changed, 94 insertions(+), 58 deletions(-)
Signature · 1 line
2.39.2
PATCH 1/2 irqdomain: Refactor __irq_domain_alloc_irqs()
From: Johan Hovold <[email protected]>
Refactor __irq_domain_alloc_irqs() so that it can be called internally while holding the irq_domain_mutex.
This will be used to fix a shared-interrupt mapping race, hence the Fixes tag.
--- kernel/irq/irqdomain.c | 88 +++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 40 deletions(-)
kernel/irq/irqdomain.c +48 -40
@@ -1464,40 +1464,12 @@ int irq_domain_alloc_irqs_hierarchy(struct irq_domain *domain,return domain->ops->alloc (domain, irq_base, nr_irqs, arg); } -/** -* __irq_domain_alloc_irqs - Allocate IRQs from domain -* @domain: domain to allocate from -* @irq_base: allocate specified IRQ number if irq_base >= 0 -* @nr_irqs: number of IRQs to allocate -* @node: NUMA node id for memory allocation -* @arg: domain specific argument -* @realloc: IRQ descriptors have already been allocated if true -* @affinity: Optional irq affinity mask for multiqueue devices -* -* Allocate IRQ numbers and initialized all data structures to support -* hierarchy IRQ domains. -* Parameter @realloc is mainly to support legacy IRQs. -* Returns error code or allocated IRQ number -* -* The whole process to setup an IRQ has been split into two steps. -* The first step, __irq_domain_alloc_irqs(), is to allocate IRQ -* descriptor and required hardware resources. The second step, -* irq_domain_activate_irq(), is to program the hardware with preallocated -* resources. In this way, it's easier to rollback when failing to -* allocate resources. -*/ -int __irq_domain_alloc_irqs (struct irq_domain *domain,int irq_base, -unsigned int nr_irqs,int node,void *arg, -bool realloc,const struct irq_affinity_desc *affinity) +static int irq_domain_alloc_irqs_locked (struct irq_domain *domain,int irq_base, +unsigned int nr_irqs,int node,void *arg, +bool realloc,const struct irq_affinity_desc *affinity) {int i, ret, virq; -if (domain ==NULL ) { - domain = irq_default_domain; -if (WARN (!domain,"domain is NULL; cannot allocate IRQ\n" )) -return -EINVAL ; - } -if (realloc && irq_base >=0 ) { virq = irq_base; }else {
@@ -1516,24 +1488,18 @@ int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base, goto out_free_desc; } -mutex_lock (&irq_domain_mutex); ret =irq_domain_alloc_irqs_hierarchy (domain, virq, nr_irqs, arg); -if (ret <0 ) { -mutex_unlock (&irq_domain_mutex); +if (ret <0 ) goto out_free_irq_data; - }for (i =0 ; i < nr_irqs; i++) { ret =irq_domain_trim_hierarchy (virq + i); -if (ret) { -mutex_unlock (&irq_domain_mutex); +if (ret) goto out_free_irq_data; - } } - +for (i =0 ; i < nr_irqs; i++)irq_domain_insert_irq (virq + i); -mutex_unlock (&irq_domain_mutex);return virq;
@@ -1544,6 +1510,48 @@ int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,return ret; } +/** +* __irq_domain_alloc_irqs - Allocate IRQs from domain +* @domain: domain to allocate from +* @irq_base: allocate specified IRQ number if irq_base >= 0 +* @nr_irqs: number of IRQs to allocate +* @node: NUMA node id for memory allocation +* @arg: domain specific argument +* @realloc: IRQ descriptors have already been allocated if true +* @affinity: Optional irq affinity mask for multiqueue devices +* +* Allocate IRQ numbers and initialized all data structures to support +* hierarchy IRQ domains. +* Parameter @realloc is mainly to support legacy IRQs. +* Returns error code or allocated IRQ number +* +* The whole process to setup an IRQ has been split into two steps. +* The first step, __irq_domain_alloc_irqs(), is to allocate IRQ +* descriptor and required hardware resources. The second step, +* irq_domain_activate_irq(), is to program the hardware with preallocated +* resources. In this way, it's easier to rollback when failing to +* allocate resources. +*/ +int __irq_domain_alloc_irqs (struct irq_domain *domain,int irq_base, +unsigned int nr_irqs,int node,void *arg, +bool realloc,const struct irq_affinity_desc *affinity) +{ +int ret; + +if (domain ==NULL ) { + domain = irq_default_domain; +if (WARN (!domain,"domain is NULL; cannot allocate IRQ\n" )) +return -EINVAL ; + } + +mutex_lock (&irq_domain_mutex); + ret =irq_domain_alloc_irqs_locked (domain, irq_base, nr_irqs, node, arg, + realloc, affinity); +mutex_unlock (&irq_domain_mutex); + +return ret; +} +/* The irq_data was moved, fix the revmap to refer to the new location */ static void irq_domain_fix_revmap (struct irq_data *d) {
Signature · 1 line
2.39.2
PATCH 2/2 irqdomain: Fix mapping-creation race
In reply to Johan HovoldFrom: Johan Hovold <[email protected]>
Parallel probing of devices that share interrupts (e.g. when a driver uses asynchronous probing) can currently result in two mappings for the same hardware interrupt to be created due to missing serialisation.
Make sure to hold the irq_domain_mutex when creating mappings so that looking for an existing mapping before creating a new one is done atomically.
--- kernel/irq/irqdomain.c | 64 ++++++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 18 deletions(-)
kernel/irq/irqdomain.c +46 -18
@@ -25,6 +25,9 @@ static DEFINE_MUTEX(irq_domain_mutex);static struct irq_domain *irq_default_domain; +static int irq_domain_alloc_irqs_locked (struct irq_domain *domain,int irq_base, +unsigned int nr_irqs,int node,void *arg, +bool realloc,const struct irq_affinity_desc *affinity);static void irq_domain_check_hierarchy (struct irq_domain *domain);struct irqchip_fwid {
@@ -703,9 +706,9 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)EXPORT_SYMBOL_GPL (irq_create_direct_mapping);#endif -static unsigned int __irq_create_mapping_affinity (struct irq_domain *domain, -irq_hw_number_t hwirq, -const struct irq_affinity_desc *affinity) +static unsigned int irq_create_mapping_affinity_locked (struct irq_domain *domain, +irq_hw_number_t hwirq, +const struct irq_affinity_desc *affinity) {struct device_node *of_node =irq_domain_get_of_node (domain);int virq;
@@ -720,7 +723,7 @@ static unsigned int __irq_create_mapping_affinity(struct irq_domain *domain,return 0 ; } -if (irq_domain_associate (domain, virq, hwirq)) { +if (irq_domain_associate_locked (domain, virq, hwirq)) {irq_free_desc (virq);return 0 ; }
@@ -756,14 +759,20 @@ unsigned int irq_create_mapping_affinity(struct irq_domain *domain,return 0 ; } +mutex_lock (&irq_domain_mutex); +/* Check if mapping already exists */ virq =irq_find_mapping (domain, hwirq);if (virq) {pr_debug ("existing mapping on virq %d\n" , virq); -return virq; + goto out; } -return __irq_create_mapping_affinity (domain, hwirq, affinity); + virq =irq_create_mapping_affinity_locked (domain, hwirq, affinity); +out: +mutex_unlock (&irq_domain_mutex); + +return virq; }EXPORT_SYMBOL_GPL (irq_create_mapping_affinity);
@@ -830,6 +839,8 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)if (WARN_ON (type & ~IRQ_TYPE_SENSE_MASK )) type &=IRQ_TYPE_SENSE_MASK ; +mutex_lock (&irq_domain_mutex); +/* * If we've already configured this interrupt, * don't do it again, or hell will break loose.
@@ -842,7 +853,7 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)* interrupt number. */ if (type ==IRQ_TYPE_NONE || type ==irq_get_trigger_type (virq)) -return virq; + goto out;/* * If the trigger type has not been set yet, then set
@@ -850,35 +861,45 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)*/ if (irq_get_trigger_type (virq) ==IRQ_TYPE_NONE ) { irq_data =irq_get_irq_data (virq); -if (!irq_data) -return 0 ; +if (!irq_data) { + virq =0 ; + goto out; + }irqd_set_trigger_type (irq_data, type); -return virq; + goto out; }pr_warn ("type mismatch, failed to map hwirq-%lu for %s!\n" , hwirq,of_node_full_name (to_of_node (fwspec->fwnode))); -return 0 ; + virq =0 ; + goto out; }if (irq_domain_is_hierarchy (domain)) { - virq =irq_domain_alloc_irqs (domain,1 ,NUMA_NO_NODE , fwspec); -if (virq <=0 ) -return 0 ; + virq =irq_domain_alloc_irqs_locked (domain,-1 ,1 ,NUMA_NO_NODE , + fwspec, false,NULL ); +if (virq <=0 ) { + virq =0 ; + goto out; + } }else {/* Create mapping */ - virq =__irq_create_mapping_affinity (domain, hwirq,NULL ); + virq =irq_create_mapping_affinity_locked (domain, hwirq,NULL );if (!virq) -return virq; + goto out; } irq_data =irq_get_irq_data (virq); -if (WARN_ON (!irq_data)) -return 0 ; +if (WARN_ON (!irq_data)) { + virq =0 ; + goto out; + }/* Store trigger type */ irqd_set_trigger_type (irq_data, type); +out: +mutex_unlock (&irq_domain_mutex);return virq; }
@@ -1910,6 +1931,13 @@ void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,irq_set_handler_data (virq, handler_data); } +static int irq_domain_alloc_irqs_locked (struct irq_domain *domain,int irq_base, +unsigned int nr_irqs,int node,void *arg, +bool realloc,const struct irq_affinity_desc *affinity) +{ +return -EINVAL ; +} +static void irq_domain_check_hierarchy (struct irq_domain *domain) { }
Signature · 1 line
2.39.2