From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754807AbcKUPfq (ORCPT ); Mon, 21 Nov 2016 10:35:46 -0500 Received: from mail.skyhub.de ([78.46.96.112]:55513 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754143AbcKUPfo (ORCPT ); Mon, 21 Nov 2016 10:35:44 -0500 Date: Mon, 21 Nov 2016 16:35:38 +0100 From: Borislav Petkov To: Peter Zijlstra Cc: Steven Rostedt , Jiri Olsa , "Paul E. McKenney" , linux-kernel@vger.kernel.org, Ingo Molnar , Josh Triplett , Andi Kleen , Jan Stancek Subject: Re: [BUG] msr-trace.h:42 suspicious rcu_dereference_check() usage! Message-ID: <20161121153538.27wegzmdv3om52xq@pd.tnic> References: <20161121005343.GB1891@krava> <20161121092850.GF3102@twins.programming.kicks-ass.net> <20161121093424.GA9814@krava> <20161121125830.GE3092@twins.programming.kicks-ass.net> <20161121091543.45f49945@gandalf.local.home> <20161121143716.GG3092@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161121143716.GG3092@twins.programming.kicks-ass.net> User-Agent: NeoMutt/20161014 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 21, 2016 at 03:37:16PM +0100, Peter Zijlstra wrote: > Yeah, but this one does a printk() when it hits the contidion it checks > for, so not tracing it would be fine I think. FWIW, there's already: static inline void notrace native_write_msr(unsigned int msr, u32 low, u32 high) { __native_write_msr_notrace(msr, low, high); if (msr_tracepoint_active(__tracepoint_write_msr)) do_trace_write_msr(msr, ((u64)high << 32 | low), 0); } so could be mirrored. > Also, Boris, why do we need to redo that rdmsr until we see that bit > set? Can't we simply do the rdmsr once and then be done with it? Hmm, so the way I read the definition of those bits - K8_INTP_C1E_ACTIVE_MASK - it looks like they get set when the cores enter halt: 28 C1eOnCmpHalt: C1E on chip multi-processing halt. Read-write. 1=When all cores of a processor have entered the halt state, the processor ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ generates an IO cycle as specified by IORd, IOMsgData, and IOMsgAddr. When this bit is set, SmiOnCmpHalt and IntPndMsg must be 0, otherwise the behavior is undefined. For revision DA-C, this bit is only supported for dual-core processors. For revision C3 and E, this bit is supported for any number of cores. See 2.4.3.3.3 [Hardware Initiated C1E]. 27 SmiOnCmpHalt: SMI on chip multi-processing halt. Read-write. 1=When all cores of the processor have entered the halt state, the ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ processor generates an SMI-trigger IO cycle as specified by IORd, IOMsgData, and IOMsgAddr. When this bit is set C1eOnCmpHalt and IntPndMsg must be 0, otherwise the behavior is undefined. The status is stored in SMMFEC4[SmiOnCmpHaltSts]. See 2.4.3.3.1 [SMI Initiated C1E]. But the thing is, we do it only once until amd_e400_c1e_detected is true. FWIW, I'd love it if we could do if (cpu_has_bug(c, X86_BUG_AMD_APIC_C1E)) but I'm afraid we need to look at that MSR at least once until we set the boolean. I could ask if that is really the case and whether we can detect it differently... In any case, I have a box here in case you want me to test patches. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.