Re: [PATCH v7 1/2] x86/split_lock: Rework the initialization flow of split lock detection

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Mon, Mar 30, 2020 at 09:26:25PM +0800, Xiaoyao Li wrote:
> On 3/29/2020 12:32 AM, Sean Christopherson wrote:
> >On Wed, Mar 25, 2020 at 11:09:23AM +0800, Xiaoyao Li wrote:
> >>  static void split_lock_init(void)
> >>  {
> >>-	if (sld_state == sld_off)
> >>-		return;
> >>-
> >>-	if (__sld_msr_set(true))
> >>-		return;
> >>-
> >>-	/*
> >>-	 * If this is anything other than the boot-cpu, you've done
> >>-	 * funny things and you get to keep whatever pieces.
> >>-	 */
> >>-	pr_warn("MSR fail -- disabled\n");
> >>-	sld_state = sld_off;
> >>+	split_lock_verify_msr(sld_state != sld_off);
> >
> >I think it'd be worth a WARN_ON() if this fails with sld_state != off.  If
> >the WRMSR fails, then presumably SLD is off when it's expected to be on.
> >The implied WARN on the unsafe WRMSR in sld_update_msr() won't fire unless
> >a task generates an #AC on a non-buggy core and then gets migrated to the
> >buggy core.  Even if the WARNs are redundant, if something is wrong it'd be
> >a lot easier for a user to triage/debug if there is a WARN in boot as
> >opposed to a runtime WARN that requires a misbehaving application and
> >scheduler behavior.
> >
> 
> IIUC, you're recommending something like below?
> 
>         WARN_ON(!split_lock_verify_msr(sld_state != sld_off) &&
> 		sld_state != sld_off);

Ya.