Hi Marc,
On 24/11/20 14:14, Marc Zyngier wrote: > @@ -680,14 +679,22 @@ int __handle_domain_irq(struct irq_domain *domain, > unsigned int hwirq, > * Some hardware gives randomly wrong interrupts. Rather > * than crashing, do something sensible. > */ > - if (unlikely(!irq || irq >= nr_irqs)) { > + if (unlikely(!irq || irq >= nr_irqs || !(desc = irq_to_desc(irq)))) { > ack_bad_irq(irq); > ret = -EINVAL; > + goto out; > + } > + > + if (IS_ENABLED(CONFIG_ARCH_WANTS_IRQ_RAW) && > + unlikely(irq_settings_is_raw(desc))) { > + generic_handle_irq_desc(desc); If I got the RCU bits right from what Thomas mentioned in https://lore.kernel.org/r/87ft5q18qs....@nanos.tec.linutronix.de https://lore.kernel.org/r/87lfewnmdz....@nanos.tec.linutronix.de then we're still missing something to notify RCU in the case the IRQ hits the idle task. All I see on our entry path is trace_hardirqs_off(); ... irq_handler() handle_domain_irq(); ... trace_hardirqs_on(); so we do currently rely on handle_domain_irq()'s irq_enter() + irq_exit() for that. rcu_irq_enter() says CONFIG_RCU_EQS_DEBUG=y can detect missing bits, but I don't get any warnings with your series on my Juno. Now, irq_enter() gives us: rcu_irq_enter(); irq_enter_rcu() raise_softirq faffery; __irq_enter() irqtime accounting; preempt count + lockdep; } __irq_enter_raw() Looking at irqentry_enter() + DEFINE_IDTENTRY_SYSVEC_SIMPLE(), I *think* we would be fine with just: rcu_irq_enter(); __irq_enter_raw(); generic_handle_irq_desc() __irq_exit_raw(); rcu_irq_exit(); I tested that and it didn't explode (though I haven't managed to make CONFIG_RCU_EQS_DEBUG squeal). Also please note RCU isn't my forte, so the above may contain traces of bullcrap. > } else { > - generic_handle_irq(irq); > + irq_enter(); > + generic_handle_irq_desc(desc); > + irq_exit(); > } > > - irq_exit(); > +out: > set_irq_regs(old_regs); > return ret; > } [...] > @@ -180,3 +182,16 @@ static inline bool irq_settings_is_hidden(struct > irq_desc *desc) > { > return desc->status_use_accessors & _IRQ_HIDDEN; > } > + > +static inline bool irq_settings_is_raw(struct irq_desc *desc) > +{ > + if (IS_ENABLED(CONFIG_ARCH_WANTS_IRQ_RAW)) > + return desc->status_use_accessors & _IRQ_RAW; > + > + /* > + * Using IRQ_RAW on architectures that don't expect it is > + * likely to be wrong. > + */ > + WARN_ON_ONCE(1); Per __handle_domain_irq()'s short-circuit evaluation, this is only entered when the above config is enabled. Perhaps a better place to check for this would be in __irq_settings_clr_and_set(). > + return false; > +}