lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 25 Jan 2017 14:58:09 +0000 From: Marc Zyngier <marc.zyngier@....com> To: linux-kernel@...r.kernel.org Cc: Andre Przywara <andre.przywara@....com>, Thomas Gleixner <tglx@...utronix.de> Subject: Re: [PATCH] irqdomain: Avoid activating interrupts more than once Hi Thomas, Any comment on that one? Thanks, M. On 17/01/17 16:00, Marc Zyngier wrote: > Since commit f3b0946d629c ("genirq/msi: Make sure PCI MSIs are > activated early"), we can end-up activating a PCI/MSI twice (once > at allocation time, and once at startup time). > > This is normally of no consequences, except that there is some > HW out there that may misbehave if activate is used more than once > (the GICv3 ITS, for example, uses the activate callback > to issue the MAPVI command, and the architecture spec says that > "If there is an existing mapping for the EventID-DeviceID > combination, behavior is UNPREDICTABLE"). > > While this could be worked around in each individual driver, it may > make more sense to tackle the issue at the core level. In order to > avoid getting in that situation, let's have a per-interrupt flag > to remember if we have already activated that interrupt or not. > > Fixes: f3b0946d629c ("genirq/msi: Make sure PCI MSIs are activated early") > Reported-by: Andre Przywara <andre.przywara@....com> > Tested-by: Andre Przywara <andre.przywara@....com> > Signed-off-by: Marc Zyngier <marc.zyngier@....com> > --- > include/linux/irq.h | 17 +++++++++++++++++ > kernel/irq/irqdomain.c | 44 ++++++++++++++++++++++++++++++-------------- > 2 files changed, 47 insertions(+), 14 deletions(-) > > diff --git a/include/linux/irq.h b/include/linux/irq.h > index e798755..39e3254 100644 > --- a/include/linux/irq.h > +++ b/include/linux/irq.h > @@ -184,6 +184,7 @@ struct irq_data { > * > * IRQD_TRIGGER_MASK - Mask for the trigger type bits > * IRQD_SETAFFINITY_PENDING - Affinity setting is pending > + * IRQD_ACTIVATED - Interrupt has already been activated > * IRQD_NO_BALANCING - Balancing disabled for this IRQ > * IRQD_PER_CPU - Interrupt is per cpu > * IRQD_AFFINITY_SET - Interrupt affinity was set > @@ -202,6 +203,7 @@ struct irq_data { > enum { > IRQD_TRIGGER_MASK = 0xf, > IRQD_SETAFFINITY_PENDING = (1 << 8), > + IRQD_ACTIVATED = (1 << 9), > IRQD_NO_BALANCING = (1 << 10), > IRQD_PER_CPU = (1 << 11), > IRQD_AFFINITY_SET = (1 << 12), > @@ -312,6 +314,21 @@ static inline bool irqd_affinity_is_managed(struct irq_data *d) > return __irqd_to_state(d) & IRQD_AFFINITY_MANAGED; > } > > +static inline bool irqd_is_activated(struct irq_data *d) > +{ > + return __irqd_to_state(d) & IRQD_ACTIVATED; > +} > + > +static inline void irqd_set_activated(struct irq_data *d) > +{ > + __irqd_to_state(d) |= IRQD_ACTIVATED; > +} > + > +static inline void irqd_clr_activated(struct irq_data *d) > +{ > + __irqd_to_state(d) &= ~IRQD_ACTIVATED; > +} > + > #undef __irqd_to_state > > static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d) > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c > index 8c0a0ae..b59e676 100644 > --- a/kernel/irq/irqdomain.c > +++ b/kernel/irq/irqdomain.c > @@ -1346,6 +1346,30 @@ void irq_domain_free_irqs_parent(struct irq_domain *domain, > } > EXPORT_SYMBOL_GPL(irq_domain_free_irqs_parent); > > +static void __irq_domain_activate_irq(struct irq_data *irq_data) > +{ > + if (irq_data && irq_data->domain) { > + struct irq_domain *domain = irq_data->domain; > + > + if (irq_data->parent_data) > + __irq_domain_activate_irq(irq_data->parent_data); > + if (domain->ops->activate) > + domain->ops->activate(domain, irq_data); > + } > +} > + > +static void __irq_domain_deactivate_irq(struct irq_data *irq_data) > +{ > + if (irq_data && irq_data->domain) { > + struct irq_domain *domain = irq_data->domain; > + > + if (domain->ops->deactivate) > + domain->ops->deactivate(domain, irq_data); > + if (irq_data->parent_data) > + __irq_domain_deactivate_irq(irq_data->parent_data); > + } > +} > + > /** > * irq_domain_activate_irq - Call domain_ops->activate recursively to activate > * interrupt > @@ -1356,13 +1380,9 @@ EXPORT_SYMBOL_GPL(irq_domain_free_irqs_parent); > */ > void irq_domain_activate_irq(struct irq_data *irq_data) > { > - if (irq_data && irq_data->domain) { > - struct irq_domain *domain = irq_data->domain; > - > - if (irq_data->parent_data) > - irq_domain_activate_irq(irq_data->parent_data); > - if (domain->ops->activate) > - domain->ops->activate(domain, irq_data); > + if (!irqd_is_activated(irq_data)) { > + __irq_domain_activate_irq(irq_data); > + irqd_set_activated(irq_data); > } > } > > @@ -1376,13 +1396,9 @@ void irq_domain_activate_irq(struct irq_data *irq_data) > */ > void irq_domain_deactivate_irq(struct irq_data *irq_data) > { > - if (irq_data && irq_data->domain) { > - struct irq_domain *domain = irq_data->domain; > - > - if (domain->ops->deactivate) > - domain->ops->deactivate(domain, irq_data); > - if (irq_data->parent_data) > - irq_domain_deactivate_irq(irq_data->parent_data); > + if (irqd_is_activated(irq_data)) { > + __irq_domain_deactivate_irq(irq_data); > + irqd_clr_activated(irq_data); > } > } > > -- Jazz is not dead. It just smells funny...
Powered by blists - more mailing lists