Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Sender: Ingo Molnar <mingo.kernel.org@gmail.com>
Date:   Thu, 12 Oct 2023 08:37:25 +0200
From:   Ingo Molnar <mingo@kernel.org>
To:     "Paul E. McKenney" <paulmck@kernel.org>
Cc:     linux-kernel@vger.kernel.org, x86@kernel.org, clm@fb.com,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com
Subject: Re: [PATCH RFC x86/nmi] Fix out-of-order nesting checks
Message-ID: <ZSeUJbZLbk2g7GC/@gmail.com>
References: <0cbff831-6e3d-431c-9830-ee65ee7787ff@paulmck-laptop>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <0cbff831-6e3d-431c-9830-ee65ee7787ff@paulmck-laptop>
Precedence: bulk


* Paul E. McKenney <paulmck@kernel.org> wrote:

> The ->idt_seq and ->recv_jiffies variables added by commit 1a3ea611fc10
> ("x86/nmi: Accumulate NMI-progress evidence in exc_nmi()") place
> the exit-time check of the bottom bit of ->idt_seq after the
> this_cpu_dec_return() that re-enables NMI nesting.  This can result in
> the following sequence of events on a given CPU in kernels built with
> CONFIG_NMI_CHECK_CPU=y:
> 
> o       An NMI arrives, and ->idt_seq is incremented to an odd number.
>         In addition, nmi_state is set to NMI_EXECUTING==1.
> 
> o       The NMI is processed.
> 
> o       The this_cpu_dec_return(nmi_state) zeroes nmi_state and returns
>         NMI_EXECUTING==1, thus opting out of the "goto nmi_restart".
> 
> o       Another NMI arrives and ->idt_seq is incremented to an even
>         number, triggering the warning.  But all is just fine, at least
>         assuming we don't get so many closely spaced NMIs that the stack
>         overflows or some such.
> 
> Experience on the fleet indicates that the MTBF of this false positive
> is about 70 years.  Or, for those who are not quite that patient, the
> MTBF appears to be about one per week per 4,000 systems.
> 
> Fix this false-positive warning by moving the "nmi_restart" label before
> the initial ->idt_seq increment/check and moving the this_cpu_dec_return()
> to follow the final ->idt_seq increment/check.  This way, all nested NMIs
> that get past the NMI_NOT_RUNNING check get a clean ->idt_seq slate.
> And if they don't get past that check, they will set nmi_state to
> NMI_LATCHED, which will cause the this_cpu_dec_return(nmi_state)
> to restart.

This looks like a sensible fix: the warning should obviously be atomic wrt. 
the no-nesting region. I've applied your fix to tip:x86/irq, as it doesn't 
seem urgent enough with a MTBF of 70 years to warrant tip:x86/urgent handling. ;-)

Thanks,

	Ingo