linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	rostedt@goodmis.org, mingo@kernel.org, joel@joelfernandes.org,
	gregkh@linuxfoundation.org, gustavo@embeddedor.com,
	tglx@linutronix.de, paulmck@kernel.org, josh@joshtriplett.org,
	mathieu.desnoyers@efficios.com, jiangshanlai@gmail.com,
	luto@kernel.org, tony.luck@intel.com, dan.carpenter@oracle.com,
	mhiramat@kernel.org, Will Deacon <will@kernel.org>,
	Petr Mladek <pmladek@suse.com>, Marc Zyngier <maz@kernel.org>
Subject: Re: [PATCH v4 02/27] hardirq/nmi: Allow nested nmi_enter()
Date: Tue, 25 Feb 2020 04:09:06 +0100	[thread overview]
Message-ID: <20200225030905.GB28329@lenoir> (raw)
In-Reply-To: <20200224161318.GG14897@hirez.programming.kicks-ass.net>

On Mon, Feb 24, 2020 at 05:13:18PM +0100, Peter Zijlstra wrote:
> Damn, true. That also means I need to fix the arm64 bits, and that's a
> little more tricky.
> 
> Something like so perhaps.. hmm?
> 
> ---
> --- a/arch/arm64/include/asm/hardirq.h
> +++ b/arch/arm64/include/asm/hardirq.h
> @@ -32,30 +32,52 @@ u64 smp_irq_stat_cpu(unsigned int cpu);
>  
>  struct nmi_ctx {
>  	u64 hcr;
> +	unsigned int cnt;
>  };
>  
>  DECLARE_PER_CPU(struct nmi_ctx, nmi_contexts);
>  
> -#define arch_nmi_enter()							\
> -	do {									\
> -		if (is_kernel_in_hyp_mode() && !in_nmi()) {			\
> -			struct nmi_ctx *nmi_ctx = this_cpu_ptr(&nmi_contexts);	\
> -			nmi_ctx->hcr = read_sysreg(hcr_el2);			\
> -			if (!(nmi_ctx->hcr & HCR_TGE)) {			\
> -				write_sysreg(nmi_ctx->hcr | HCR_TGE, hcr_el2);	\
> -				isb();						\
> -			}							\
> -		}								\
> -	} while (0)
> +#define arch_nmi_enter()						\
> +do {									\
> +	struct nmi_ctx *___ctx;						\
> +	unsigned int ___cnt;						\
> +									\
> +	if (!is_kernel_in_hyp_mode() || in_nmi())			\
> +		break;							\
> +									\
> +	___ctx = this_cpu_ptr(&nmi_contexts);				\
> +	___cnt = ___ctx->cnt;						\
> +	if (!(___cnt & 1) && __cnt) {					\
> +		___ctx->cnt += 2;					\
> +		break;							\
> +	}								\
> +									\
> +	___ctx->cnt |= 1;						\
> +	barrier();							\
> +	nmi_ctx->hcr = read_sysreg(hcr_el2);				\
> +	if (!(nmi_ctx->hcr & HCR_TGE)) {				\
> +		write_sysreg(nmi_ctx->hcr | HCR_TGE, hcr_el2);		\
> +		isb();							\
> +	}								\
> +	barrier();							\

Suppose the first NMI is interrupted here. nmi_ctx->hcr has HCR_TGE unset.
The new NMI is going to overwrite nmi_ctx->hcr with HCR_TGE set. Then the
first NMI will not restore the correct value upon arch_nmi_exit().

So perhaps the below, but I bet I overlooked something obvious.

#define arch_nmi_enter()                                             \
do {                                                                 \
     struct nmi_ctx *___ctx;                                         \
     u64 ___hcr;                                                     \
                                                                     \
     if (!is_kernel_in_hyp_mode())                                   \
             break;                                                  \
                                                                     \
     ___ctx = this_cpu_ptr(&nmi_contexts);                           \
     if (___ctx->cnt) {                                              \
             ___ctx->cnt++;                                          \
             break;                                                  \
     }                                                               \
                                                                     \
     ___hcr = read_sysreg(hcr_el2);                                  \
     if (!(___hcr & HCR_TGE)) {                                      \
             write_sysreg(___hcr | HCR_TGE, hcr_el2);                \
             isb();                                                  \
     }                                                               \
     ___ctx->cnt = 1;                                                \
     barrier();                                                      \
     ___ctx->hcr = ___hcr;                                           \
} while (0)

#define arch_nmi_exit()                                         \
do {                                                            \
        struct nmi_ctx *___ctx;                                 \
        u64 ___hcr;                                             \
                                                                \
        if (!is_kernel_in_hyp_mode())                           \
            break;                                              \
                                                                \
        ___ctx = this_cpu_ptr(&nmi_contexts);                   \
	___hcr = nmi_ctx->hcr;                                  \
        barrier();                                              \
        --___ctx->cnt;                                          \
	barrier();                                              \
	if (!___ctx->cnt && !(___hcr & HCR_TGE))                  \
            write_sysreg(___hcr, hcr_el2);                       \
} while (0)

  reply	other threads:[~2020-02-25  3:09 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-21 13:34 [PATCH v4 00/27] tracing vs world Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 01/27] lockdep: Teach lockdep about "USED" <- "IN-NMI" inversions Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 15:08   ` Steven Rostedt
2020-02-21 15:08     ` Steven Rostedt
2020-02-21 20:25     ` Peter Zijlstra
2020-02-21 20:28       ` Steven Rostedt
2020-02-21 22:01       ` Frederic Weisbecker
2020-02-22  3:08   ` Joel Fernandes
2020-02-24 10:10     ` Peter Zijlstra
2020-02-25  2:12       ` Joel Fernandes
2020-02-21 13:34 ` [PATCH v4 02/27] hardirq/nmi: Allow nested nmi_enter() Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 22:21   ` Frederic Weisbecker
2020-02-24 12:13     ` Petr Mladek
2020-02-25  1:30       ` Frederic Weisbecker
2020-02-24 16:13     ` Peter Zijlstra
2020-02-25  3:09       ` Frederic Weisbecker [this message]
2020-02-25 15:41         ` Peter Zijlstra
2020-02-25 16:21           ` Frederic Weisbecker
2020-02-25 22:10           ` Frederic Weisbecker
2020-02-27  9:10             ` Peter Zijlstra
2020-02-27 13:34               ` Frederic Weisbecker
2020-02-27 13:34                 ` Frederic Weisbecker
2020-02-21 13:34 ` [PATCH v4 03/27] x86/entry: Flip _TIF_SIGPENDING and _TIF_NOTIFY_RESUME handling Peter Zijlstra
2020-02-21 16:14   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 04/27] x86/mce: Delete ist_begin_non_atomic() Peter Zijlstra
2020-02-21 19:07   ` Andy Lutomirski
2020-02-21 23:40   ` Frederic Weisbecker
2020-02-21 13:34 ` [PATCH v4 05/27] x86: Replace ist_enter() with nmi_enter() Peter Zijlstra
2020-02-21 19:05   ` Andy Lutomirski
2020-02-21 20:22     ` Peter Zijlstra
2020-02-24 10:43       ` Peter Zijlstra
2020-02-24 16:27         ` Steven Rostedt
2020-02-24 16:34           ` Peter Zijlstra
2020-02-24 16:47             ` Steven Rostedt
2020-02-24 21:31               ` Peter Zijlstra
2020-02-24 22:02                 ` Steven Rostedt
2020-02-26 10:25                   ` Peter Zijlstra
2020-02-26 13:16                     ` Peter Zijlstra
2020-02-26 10:27                   ` Peter Zijlstra
2020-02-26 10:27                     ` Peter Zijlstra
2020-02-26 15:20                     ` Steven Rostedt
2020-03-07  1:53                     ` Masami Hiramatsu
2020-03-07  1:53                       ` Masami Hiramatsu
2020-02-21 13:34 ` [PATCH v4 06/27] x86/doublefault: Remove memmove() call Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 19:10   ` Andy Lutomirski
2020-02-21 13:34 ` [PATCH v4 07/27] rcu: Make RCU IRQ enter/exit functions rely on in_nmi() Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-26  0:23   ` Frederic Weisbecker
2020-02-21 13:34 ` [PATCH v4 08/27] rcu/kprobes: Comment why rcu_nmi_enter() is marked NOKPROBE Peter Zijlstra
2020-02-26  0:27   ` Frederic Weisbecker
2020-02-21 13:34 ` [PATCH v4 09/27] rcu: Rename rcu_irq_{enter,exit}_irqson() Peter Zijlstra
2020-02-21 20:21   ` Steven Rostedt
2020-02-24 10:24     ` Peter Zijlstra
2020-02-26  0:35   ` Frederic Weisbecker
2020-02-21 13:34 ` [PATCH v4 10/27] rcu: Mark rcu_dynticks_curr_cpu_in_eqs() inline Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 11/27] rcu,tracing: Create trace_rcu_{enter,exit}() Peter Zijlstra
2020-03-06 11:50   ` Peter Zijlstra
2020-03-06 12:40     ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 12/27] sched,rcu,tracing: Avoid tracing before in_nmi() is correct Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 13/27] x86,tracing: Add comments to do_nmi() Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 14/27] perf,tracing: Prepare the perf-trace interface for RCU changes Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 15/27] tracing: Employ trace_rcu_{enter,exit}() Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 16/27] tracing: Remove regular RCU context for _rcuidle tracepoints (again) Peter Zijlstra
2020-03-06 10:43   ` Peter Zijlstra
2020-03-06 11:31     ` Peter Zijlstra
2020-03-06 11:31       ` Peter Zijlstra
2020-03-06 15:51       ` Alexei Starovoitov
2020-03-06 16:04         ` Mathieu Desnoyers
2020-03-06 17:55           ` Steven Rostedt
2020-03-06 18:45             ` Joel Fernandes
2020-03-06 18:59               ` Steven Rostedt
2020-03-06 19:14                 ` Joel Fernandes
2020-03-06 19:14                   ` Joel Fernandes
2020-03-06 20:22             ` Mathieu Desnoyers
2020-03-06 20:45               ` Steven Rostedt
2020-03-06 20:45                 ` Steven Rostedt
2020-03-06 20:55                 ` Mathieu Desnoyers
2020-03-06 21:06                   ` Steven Rostedt
2020-03-06 23:10                   ` Alexei Starovoitov
2020-03-06 17:21         ` Joel Fernandes
2020-02-21 13:34 ` [PATCH v4 17/27] perf,tracing: Allow function tracing when !RCU Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 18/27] x86/int3: Ensure that poke_int3_handler() is not traced Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 19/27] locking/atomics, kcsan: Add KCSAN instrumentation Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 20/27] asm-generic/atomic: Use __always_inline for pure wrappers Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 21/27] asm-generic/atomic: Use __always_inline for fallback wrappers Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 22/27] compiler: Simple READ/WRITE_ONCE() implementations Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 23/27] locking/atomics: Flip fallbacks and instrumentation Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 24/27] x86/int3: Avoid atomic instrumentation Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 25/27] lib/bsearch: Provide __always_inline variant Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 26/27] x86/int3: Inline bsearch() Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra
2020-02-21 13:34 ` [PATCH v4 27/27] x86/int3: Ensure that poke_int3_handler() is not sanitized Peter Zijlstra
2020-02-21 13:34   ` Peter Zijlstra

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=20200225030905.GB28329@lenoir \
    --to=frederic@kernel.org \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=gustavo@embeddedor.com \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=maz@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).