Re: [RFC PATCH 02/16] x86/split_lock: Handle #AC exception for split lock in kernel mode

[Fenghua Yu <fenghua.yu@intel.com>]

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