From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by aws-us-west-2-korg-lkml-1.web.codeaurora.org (Postfix) with ESMTP id 89855C07D5F for ; Mon, 11 Jun 2018 17:57:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 49A9B208B2 for ; Mon, 11 Jun 2018 17:57:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 49A9B208B2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934129AbeFKR50 (ORCPT ); Mon, 11 Jun 2018 13:57:26 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:58458 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932292AbeFKR5Y (ORCPT ); Mon, 11 Jun 2018 13:57:24 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 01EC8BD9E; Mon, 11 Jun 2018 17:57:24 +0000 (UTC) Received: from darcari.bos.csb (dhcp-17-203.bos.redhat.com [10.18.17.203]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1B84584438; Mon, 11 Jun 2018 17:57:23 +0000 (UTC) Subject: Re: [PATCH] perf/x86: read the FREEZE_WHILE_SMM bit during boot From: David Arcari To: Peter Zijlstra Cc: linux-kernel@vger.kernel.org, Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Andi Kleen , Kan Liang , Jiri Olsa , Donald Zickus , Prarit Bhargava , Jerry Hoemann References: <1528050223-144925-1-git-send-email-darcari@redhat.com> <20180604082414.GO12217@hirez.programming.kicks-ass.net> Organization: Red Hat Message-ID: Date: Mon, 11 Jun 2018 13:57:22 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 11 Jun 2018 17:57:24 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 11 Jun 2018 17:57:24 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'darcari@redhat.com' RCPT:'' Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/04/2018 10:12 AM, David Arcari wrote: > On 06/04/2018 04:24 AM, Peter Zijlstra wrote: >> On Sun, Jun 03, 2018 at 02:23:43PM -0400, David Arcari wrote: >>> On some systems pressing the external NMI button is now failing to inject >>> an NMI 5-10% of the time. This causes confusion for a user that expects >>> the NMI to dump the system. >>> >>> Commit 6089327f5424 ("perf/x86: Add sysfs entry to freeze counters on SMI") >>> does not read the firmware setting of the FREEZE_WHILE_SMM bit and will >>> always clear it when the PMU is initialized. As a result the performance >>> counters will always run and that greatly expands the race in which >>> external NMI will not be processed if a local NMI is already being >>> processed. >>> >>> One option is to change default_do_nmi(). The code snippet below shows the >>> relevant portion of a patch that resolves the issue, but it is problematic >>> from a performance perspective and was dismissed. >>> >>> -345,7 +345,17 @@ static void default_do_nmi(struct pt_regs *regs) >>> */ >>> if (handled > 1) >>> __this_cpu_write(swallow_nmi, true); >>> - return; >>> + >>> + /* >>> + * Unfortunately, there is a race condition which can >>> + * result in a missing an external NMI. Typically, an >>> + * external NMI is processed on cpu 0. Therefore, on >>> + * cpu 0 check for an external NMI before returning. >>> + */ >>> + if (smp_processor_id() || >>> + (x86_platform.get_nmi_reason() & NMI_REASON_MASK) == 0) { >>> + return; >>> + } >>> } >>> >>> Ultimately, the issue can be resolved by storing the default firmware >>> setting of FREEZE_WHILE_SMM before initializing the PMU. >> >> I'm sorry, I know it's Monday morning, but what?! I really don't >> understand anything you write there. >> >> Maybe if you explain the race and how your proposed fix closes it things >> will make sense. The above refers to too many things not here. >> > > Sorry. > > default_do_nmi() will process both perf events (local interrupts) as well as > external interrupts (such as the NMI button). The handler is coded such that > if a local interrupt occurs, no check is made for an external interrupt. > Therefore, if the two interrupts occur simultaneously, the external interrupt > is lost. > > The code above, which was ultimately discounted, attempts to avoid this > scenario with as little performance impact as possible by reading the register > without the spinlock for cpu 0 only (currently only cpu 0 can handle an > external NMI, I verified this on my system by testing the NMI button with > cpu 0 offline). > > The code above is problematic for a number of reasons not the least of which > is performance. Furthermore, I don't see a less intrusive solution wrt > do_default_nmi(). > > Upstream 6089327f5424 ("perf/x86: Add sysfs entry to freeze counters on SMI") > appears to have made it relatively easy to hit this race condition. On some > systems, this commit has resulted in a change to the default firmware setting > of DEBUGCTLMSR_FREEZE_IN_SMM_BIT (it is now cleared by the OS by default). > > With this bit cleared, the following situation occurs: > > 1) external NMI - due to io check > 2) long duration SMI (counters do not freeze) > 3) NMI handler runs and misattributes interrupt to perf event > > Ultimately, my solution was to restore the previous behavior by reading and > storing the firmware setting of the bit rather than to always clear it. > Hi Peter, Have you had a chance to take a look at this? Thanks, -Dave