Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 23 Jun 2018 08:05:21 -0700
From:   Fenghua Yu <fenghua.yu@intel.com>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     Fenghua Yu <fenghua.yu@intel.com>, Ingo Molnar <mingo@elte.hu>,
        "H. Peter Anvin" <hpa@linux.intel.com>,
        Ashok Raj <ashok.raj@intel.com>,
        Dave Hansen <dave.hansen@intel.com>,
        Rafael Wysocki <rafael.j.wysocki@intel.com>,
        Tony Luck <tony.luck@intel.com>,
        Alan Cox <alan@linux.intel.com>,
        Ravi V Shankar <ravi.v.shankar@intel.com>,
        Arjan van de Ven <arjan@infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>
Subject: Re: [RFC PATCH 02/16] x86/split_lock: Handle #AC exception for split
 lock in kernel mode
Message-ID: <20180623150521.GG18979@romley-ivt3.sc.intel.com>
References: <1527435965-202085-1-git-send-email-fenghua.yu@intel.com>
 <1527435965-202085-3-git-send-email-fenghua.yu@intel.com>
 <alpine.DEB.2.21.1806212301590.1591@nanos.tec.linutronix.de>
 <20180623042033.GF18979@romley-ivt3.sc.intel.com>
 <alpine.DEB.2.21.1806231108540.8650@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.21.1806231108540.8650@nanos.tec.linutronix.de>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Jun 23, 2018 at 11:17:03AM +0200, Thomas Gleixner wrote:
> On Fri, 22 Jun 2018, Fenghua Yu wrote:
> > On Fri, Jun 22, 2018 at 12:49:00PM +0200, Thomas Gleixner wrote:
> > > On Sun, 27 May 2018, Fenghua Yu wrote:
> > > > +static void wait_for_reexecution(void)
> > > > +{
> > > > +	while (time_before(jiffies, disable_split_lock_jiffies +
> > > > +			   reenable_split_lock_delay))
> > > > +		cpu_relax();
> > > > +}
> > > > +
> > > > +/*
> > > > + * TEST_CTL MSR is shared among threads on the same core. To simplify
> > > > + * situation, disable_split_lock_jiffies is global instead of per core.
> > > 
> > > This patch surely earns extra points in the trainwreck engineering contest,
> > > but that's not taking place on LKML.
> > > 
> > > The whole thing is simply:
> > > 
> > > handle_ac()
> > > {
> > > 	if (user_mode(regs)) {
> > > 		 do_trap(AC, SIGBUS, ...);
> > > 	} else {
> > > 		disable_ac_on_local_cpu();
> > > 		WARN_ONCE(1);
> > > 	}
> > > }
> > 
> > Should I add kernel parameter or control knob to opt-out the feature?
> 
> A simple command line option 'acoff' or something more sensible should be
> ok. No sysfs knobs or whatever please. The Kconfig option is not required
> either.

Ok. I will have a command line option.

BTW, I have a Kconfig option to enable split lock test in kernel mode in
patch #15. Are the Kconfig option and the kernel test code still needed
in next version?

> 
> > I'm afraid firmware may hang system after handling split lock if the
> > feature is enabled by kernel, e.g. "reboot" hits split lock in firmware
> > and firmware hangs the system after handling #AC.
> 
> Have you observed the problem in reality? I mean why would 'reboot' be the
> critical path? I'd rather expect that EFI callbacks or SMM 'value add'
> would trip over it.
> 
> Vs. reboot. If that is the only problem then we might just have to clear
> #AC enable before issuing it, but that does not need to be part of the
> initial patch set. Its an orthogonal issue.

Yes, I do see a real firmware hang after hitting and handling a split lock
in firmware during "reboot" in one simulation test environment. Apprantly
the split lock (and alignment access) is treated as a failure in firmware.

This real case triggered my concern that split lock in any future
firmware may happen in any path including run time service, S3/S4/S5,
hotplug. If we don't have opt-out option or something similar, system hang
from split lock in firmware can be a blocking issue on some platforms. If
that happens, bisect always finds the split lock patch to blame.

Thanks.

-Fenghua