From: Pingfan Liu <kernelfans@gmail.com> To: linux-arm-kernel@lists.infradead.org Cc: Pingfan Liu <kernelfans@gmail.com>, Mark Rutland <mark.rutland@arm.com>, "Paul E . McKenney" <paulmck@kernel.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Joey Gouly <joey.gouly@arm.com>, Sami Tolvanen <samitolvanen@google.com>, Julien Thierry <julien.thierry@arm.com>, Yuichi Ito <ito-yuichi@fujitsu.com>, rcu@vger.kernel.org Subject: [PATCHv3 1/4] arm64: entry: judge nmi ealier to avoid deadlock in RCU Date: Tue, 16 Nov 2021 16:24:47 +0800 [thread overview] Message-ID: <20211116082450.10357-2-kernelfans@gmail.com> (raw) In-Reply-To: <20211116082450.10357-1-kernelfans@gmail.com> Linux kernel places strict semantics between NMI and maskable interrupt. So does the RCU component, else deadlock may happen. But the current arm64 entry code can partially breach this rule through calling rcu_nmi_enter(). *** how a deadlock can happen if NMI mistaken as IRQ *** rcu_nmi_enter() { if (rcu_dynticks_curr_cpu_in_eqs()) { if (!in_nmi()) rcu_dynticks_task_exit(); ... if (!in_nmi()) { instrumentation_begin(); rcu_cleanup_after_idle(); instrumentation_end(); } ... } else if (!in_nmi()) { instrumentation_begin(); rcu_irq_enter_check_tick(); } } If a NMI is mistaken as a maskable interrupt, rcu_irq_enter_check_tick() can hit a deadlock, which is demonstrated by the following scenario: note_gp_changes() runs in a task context { local_irq_save(flags); // this protects against irq, but not NMI rnp = rdp->mynode; ... raw_spin_trylock_rcu_node(rnp) -------> broken in by (p)NMI, without taking __nmi_enter() rcu_nmi_enter() ->__rcu_irq_enter_check_tick() ->raw_spin_lock_rcu_node(rdp->mynode); deadlock happens!!! } *** On arm64, how pNMI mistaken as IRQ *** On arm64, pNMI is an analogue to NMI. In essence, it is a higher priority interrupt but not disabled by local_irq_disable(). In current implementation 1) If a pNMI from a context where IRQs were masked, it can be recognized as nmi, and calls __nmi_enter() immediately. This is no problem. 2) But it causes trouble if a pNMI from a context where IRQs were unmasked, and temporarily regarded as maskable interrupt. It is not treated as NMI, i.e. calling nmi_enter() until reading from GIC. __el1_irq() { irq_enter_rcu() ----> hit the deadlock bug gic_handle_nmi() nmi_enter() nmi_exit() irq_exit_rcu() } *** Remedy *** If the irqchip level exposes an interface for detecting pNMI to arch level code, it can meet the requirement at this early stage. That is the interface (*interrupt_is_nmi)() in this patch. Signed-off-by: Pingfan Liu <kernelfans@gmail.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Paul E. McKenney <paulmck@kernel.org> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Marc Zyngier <maz@kernel.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Joey Gouly <joey.gouly@arm.com> Cc: Sami Tolvanen <samitolvanen@google.com> Cc: Julien Thierry <julien.thierry@arm.com> Cc: Yuichi Ito <ito-yuichi@fujitsu.com> Cc: rcu@vger.kernel.org To: linux-arm-kernel@lists.infradead.org --- arch/arm64/include/asm/irq.h | 1 + arch/arm64/kernel/entry-common.c | 10 +++++++++- arch/arm64/kernel/irq.c | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h index fac08e18bcd5..f3eb13bfa65e 100644 --- a/arch/arm64/include/asm/irq.h +++ b/arch/arm64/include/asm/irq.h @@ -11,6 +11,7 @@ struct pt_regs; int set_handle_irq(void (*handle_irq)(struct pt_regs *)); #define set_handle_irq set_handle_irq int set_handle_fiq(void (*handle_fiq)(struct pt_regs *)); +int set_nmi_discriminator(bool (*discriminator)(void)); static inline int nr_legacy_irqs(void) { diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c index f7408edf8571..5a1a5dd66d04 100644 --- a/arch/arm64/kernel/entry-common.c +++ b/arch/arm64/kernel/entry-common.c @@ -260,6 +260,14 @@ static void do_interrupt_handler(struct pt_regs *regs, extern void (*handle_arch_irq)(struct pt_regs *); extern void (*handle_arch_fiq)(struct pt_regs *); +extern bool (*interrupt_is_nmi)(void); + +static inline bool is_in_pnmi(struct pt_regs *regs) +{ + if (!interrupts_enabled(regs) || (*interrupt_is_nmi)()) + return true; + return false; +} static void noinstr __panic_unhandled(struct pt_regs *regs, const char *vector, unsigned int esr) @@ -454,7 +462,7 @@ static void noinstr el1_interrupt(struct pt_regs *regs, { write_sysreg(DAIF_PROCCTX_NOIRQ, daif); - if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs)) + if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && is_in_pnmi(regs)) __el1_pnmi(regs, handler); else __el1_irq(regs, handler); diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index bda49430c9ea..fabed09ed966 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -81,8 +81,14 @@ static void default_handle_fiq(struct pt_regs *regs) panic("FIQ taken without a root FIQ handler\n"); } +static bool default_nmi_discriminator(void) +{ + return false; +} + void (*handle_arch_irq)(struct pt_regs *) __ro_after_init = default_handle_irq; void (*handle_arch_fiq)(struct pt_regs *) __ro_after_init = default_handle_fiq; +bool (*interrupt_is_nmi)(void) __ro_after_init = default_nmi_discriminator; int __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) { @@ -104,6 +110,18 @@ int __init set_handle_fiq(void (*handle_fiq)(struct pt_regs *)) return 0; } +#if IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) + +int __init set_nmi_discriminator(bool (*discriminator)(void)) +{ + if (interrupt_is_nmi != default_nmi_discriminator) + return -EBUSY; + + interrupt_is_nmi = discriminator; + return 0; +} +#endif + void __init init_IRQ(void) { init_irq_stacks(); -- 2.31.1
WARNING: multiple messages have this Message-ID (diff)
From: Pingfan Liu <kernelfans@gmail.com> To: linux-arm-kernel@lists.infradead.org Cc: Pingfan Liu <kernelfans@gmail.com>, Mark Rutland <mark.rutland@arm.com>, "Paul E . McKenney" <paulmck@kernel.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Joey Gouly <joey.gouly@arm.com>, Sami Tolvanen <samitolvanen@google.com>, Julien Thierry <julien.thierry@arm.com>, Yuichi Ito <ito-yuichi@fujitsu.com>, rcu@vger.kernel.org Subject: [PATCHv3 1/4] arm64: entry: judge nmi ealier to avoid deadlock in RCU Date: Tue, 16 Nov 2021 16:24:47 +0800 [thread overview] Message-ID: <20211116082450.10357-2-kernelfans@gmail.com> (raw) In-Reply-To: <20211116082450.10357-1-kernelfans@gmail.com> Linux kernel places strict semantics between NMI and maskable interrupt. So does the RCU component, else deadlock may happen. But the current arm64 entry code can partially breach this rule through calling rcu_nmi_enter(). *** how a deadlock can happen if NMI mistaken as IRQ *** rcu_nmi_enter() { if (rcu_dynticks_curr_cpu_in_eqs()) { if (!in_nmi()) rcu_dynticks_task_exit(); ... if (!in_nmi()) { instrumentation_begin(); rcu_cleanup_after_idle(); instrumentation_end(); } ... } else if (!in_nmi()) { instrumentation_begin(); rcu_irq_enter_check_tick(); } } If a NMI is mistaken as a maskable interrupt, rcu_irq_enter_check_tick() can hit a deadlock, which is demonstrated by the following scenario: note_gp_changes() runs in a task context { local_irq_save(flags); // this protects against irq, but not NMI rnp = rdp->mynode; ... raw_spin_trylock_rcu_node(rnp) -------> broken in by (p)NMI, without taking __nmi_enter() rcu_nmi_enter() ->__rcu_irq_enter_check_tick() ->raw_spin_lock_rcu_node(rdp->mynode); deadlock happens!!! } *** On arm64, how pNMI mistaken as IRQ *** On arm64, pNMI is an analogue to NMI. In essence, it is a higher priority interrupt but not disabled by local_irq_disable(). In current implementation 1) If a pNMI from a context where IRQs were masked, it can be recognized as nmi, and calls __nmi_enter() immediately. This is no problem. 2) But it causes trouble if a pNMI from a context where IRQs were unmasked, and temporarily regarded as maskable interrupt. It is not treated as NMI, i.e. calling nmi_enter() until reading from GIC. __el1_irq() { irq_enter_rcu() ----> hit the deadlock bug gic_handle_nmi() nmi_enter() nmi_exit() irq_exit_rcu() } *** Remedy *** If the irqchip level exposes an interface for detecting pNMI to arch level code, it can meet the requirement at this early stage. That is the interface (*interrupt_is_nmi)() in this patch. Signed-off-by: Pingfan Liu <kernelfans@gmail.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Paul E. McKenney <paulmck@kernel.org> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Marc Zyngier <maz@kernel.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Joey Gouly <joey.gouly@arm.com> Cc: Sami Tolvanen <samitolvanen@google.com> Cc: Julien Thierry <julien.thierry@arm.com> Cc: Yuichi Ito <ito-yuichi@fujitsu.com> Cc: rcu@vger.kernel.org To: linux-arm-kernel@lists.infradead.org --- arch/arm64/include/asm/irq.h | 1 + arch/arm64/kernel/entry-common.c | 10 +++++++++- arch/arm64/kernel/irq.c | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h index fac08e18bcd5..f3eb13bfa65e 100644 --- a/arch/arm64/include/asm/irq.h +++ b/arch/arm64/include/asm/irq.h @@ -11,6 +11,7 @@ struct pt_regs; int set_handle_irq(void (*handle_irq)(struct pt_regs *)); #define set_handle_irq set_handle_irq int set_handle_fiq(void (*handle_fiq)(struct pt_regs *)); +int set_nmi_discriminator(bool (*discriminator)(void)); static inline int nr_legacy_irqs(void) { diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c index f7408edf8571..5a1a5dd66d04 100644 --- a/arch/arm64/kernel/entry-common.c +++ b/arch/arm64/kernel/entry-common.c @@ -260,6 +260,14 @@ static void do_interrupt_handler(struct pt_regs *regs, extern void (*handle_arch_irq)(struct pt_regs *); extern void (*handle_arch_fiq)(struct pt_regs *); +extern bool (*interrupt_is_nmi)(void); + +static inline bool is_in_pnmi(struct pt_regs *regs) +{ + if (!interrupts_enabled(regs) || (*interrupt_is_nmi)()) + return true; + return false; +} static void noinstr __panic_unhandled(struct pt_regs *regs, const char *vector, unsigned int esr) @@ -454,7 +462,7 @@ static void noinstr el1_interrupt(struct pt_regs *regs, { write_sysreg(DAIF_PROCCTX_NOIRQ, daif); - if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && !interrupts_enabled(regs)) + if (IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) && is_in_pnmi(regs)) __el1_pnmi(regs, handler); else __el1_irq(regs, handler); diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c index bda49430c9ea..fabed09ed966 100644 --- a/arch/arm64/kernel/irq.c +++ b/arch/arm64/kernel/irq.c @@ -81,8 +81,14 @@ static void default_handle_fiq(struct pt_regs *regs) panic("FIQ taken without a root FIQ handler\n"); } +static bool default_nmi_discriminator(void) +{ + return false; +} + void (*handle_arch_irq)(struct pt_regs *) __ro_after_init = default_handle_irq; void (*handle_arch_fiq)(struct pt_regs *) __ro_after_init = default_handle_fiq; +bool (*interrupt_is_nmi)(void) __ro_after_init = default_nmi_discriminator; int __init set_handle_irq(void (*handle_irq)(struct pt_regs *)) { @@ -104,6 +110,18 @@ int __init set_handle_fiq(void (*handle_fiq)(struct pt_regs *)) return 0; } +#if IS_ENABLED(CONFIG_ARM64_PSEUDO_NMI) + +int __init set_nmi_discriminator(bool (*discriminator)(void)) +{ + if (interrupt_is_nmi != default_nmi_discriminator) + return -EBUSY; + + interrupt_is_nmi = discriminator; + return 0; +} +#endif + void __init init_IRQ(void) { init_irq_stacks(); -- 2.31.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-11-16 8:25 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-16 8:24 [PATCHv3 0/4] arm64: Fixes RCU deadlock due to a mistaken Pingfan Liu 2021-11-16 8:24 ` Pingfan Liu 2021-11-16 8:24 ` Pingfan Liu [this message] 2021-11-16 8:24 ` [PATCHv3 1/4] arm64: entry: judge nmi ealier to avoid deadlock in RCU Pingfan Liu 2021-11-17 11:38 ` Mark Rutland 2021-11-17 11:38 ` Mark Rutland 2021-11-19 2:01 ` Pingfan Liu 2021-11-19 2:01 ` Pingfan Liu 2021-11-19 14:04 ` Mark Rutland 2021-11-19 14:04 ` Mark Rutland 2021-11-16 8:24 ` [PATCHv3 2/4] arm64: entry: distinguish pNMI earlier in el0 interrupt Pingfan Liu 2021-11-16 8:24 ` Pingfan Liu 2021-11-16 8:24 ` [PATCHv3 3/4] irqchip: GICv3: expose pNMI discriminator Pingfan Liu 2021-11-16 8:24 ` Pingfan Liu 2021-11-16 9:53 ` Marc Zyngier 2021-11-16 9:53 ` Marc Zyngier 2021-11-17 10:16 ` Pingfan Liu 2021-11-17 10:16 ` Pingfan Liu 2021-11-17 11:01 ` Marc Zyngier 2021-11-17 11:01 ` Marc Zyngier 2021-11-19 2:38 ` Pingfan Liu 2021-11-19 2:38 ` Pingfan Liu 2021-11-16 8:24 ` [PATCHv3 4/4] arm64: entry: remove pNMI judgement in __el1_interrupt() path Pingfan Liu 2021-11-16 8:24 ` Pingfan Liu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20211116082450.10357-2-kernelfans@gmail.com \ --to=kernelfans@gmail.com \ --cc=catalin.marinas@arm.com \ --cc=ito-yuichi@fujitsu.com \ --cc=joey.gouly@arm.com \ --cc=julien.thierry@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=mark.rutland@arm.com \ --cc=maz@kernel.org \ --cc=paulmck@kernel.org \ --cc=rcu@vger.kernel.org \ --cc=samitolvanen@google.com \ --cc=tglx@linutronix.de \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.