Date:   Wed, 10 Apr 2019 08:31:31 +0200 (CEST)
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Fenghua Yu <fenghua.yu@intel.com>
cc:     Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        H Peter Anvin <hpa@zytor.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Ashok Raj <ashok.raj@intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Kalle Valo <kvalo@codeaurora.org>,
        Xiaoyao Li <xiaoyao.li@intel.com>,
        Michael Chan <michael.chan@broadcom.com>,
        Ravi V Shankar <ravi.v.shankar@intel.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        x86 <x86@kernel.org>, linux-wireless@vger.kernel.org,
        netdev@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH v6 13/20] x86/split_lock: Enable split lock detection by
 default
In-Reply-To: <20190410000229.GA209676@romley-ivt3.sc.intel.com>
Message-ID: <alpine.DEB.2.21.1904100827530.2020@nanos.tec.linutronix.de>
References: <1554326526-172295-1-git-send-email-fenghua.yu@intel.com> <1554326526-172295-14-git-send-email-fenghua.yu@intel.com> <alpine.DEB.2.21.1904041932500.1802@nanos.tec.linutronix.de> <20190410000229.GA209676@romley-ivt3.sc.intel.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Tue, 9 Apr 2019, Fenghua Yu wrote:
> On Thu, Apr 04, 2019 at 08:07:57PM +0200, Thomas Gleixner wrote:
> > On Wed, 3 Apr 2019, Fenghua Yu wrote:
> > >  static void early_init_intel(struct cpuinfo_x86 *c)
> > >  {
> > >  	u64 misc_enable;
> > >  
> > > +	init_split_lock_detect(c);
> > 
> > so we have in early boot:
> > 
> > 	early_cpu_init()
> > 	  early_identify_cpu()
> > 	    this_cpu->c_early_init(c)
> > 	      early_init_intel() {
> > 	        init_split_lock_detect();
> > 	      }	
> >             ....
> >             cpu_set_core_cap_bits(c)
> > 	       set(FEATURE_SPLIT_LOCK)
> > 
> > I don't have to understand how init_split_lock_detect() will magically see
> > the feature bit which gets set afterwards, right? 
> 
> early_init_intel() is called twice on the boot CPU. Besides it's called
> in earl_cpu_init(), it's also called in:
> 	identify_boot_cpu()
> 		identify_cpu()
> 			init_intel()
> 				early_init_intel()
> 					init_split_lock_detect();
> 
> It's true that init_split_lock_detect() doesn't see the feature bit when
> it's called for the first time in early_cpu_init(). But it sees the feature
> bit when it's called for the second time in identify_boot_cpu().

That's hideous, really. 

> So is init_split_lock_detect() in the right place?

You're not seriously asking that?

It's obviously not the right place. We are not placing calls at random
points just because they happen to work by chance.

Thanks,

	tglx