Re: [PATCH v6 10/20] x86/split_lock: Handle #AC exception for split lock

From: Fenghua Yu
Date: Thu Apr 04 2019 - 18:56:56 EST


On Thu, Apr 04, 2019 at 07:31:59PM +0200, Thomas Gleixner wrote:
> On Wed, 3 Apr 2019, Fenghua Yu wrote:
> > +dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code)
> > +{
> > + unsigned int trapnr = X86_TRAP_AC;
> > + char str[] = "alignment check";
> > + int signr = SIGBUS;
> > +
> > + RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> > +
> > + /*
> > + * WARN*()s end up here; fix them up before we call the
> > + * notifier chain.
> > + */
>
> How exactly is WARN*() ending up here?
>
> > + if (!user_mode(regs) && fixup_bug(regs, trapnr))
>
> And that fixup_bug() check does what?
>
> int fixup_bug(struct pt_regs *regs, int trapnr)
> {
> if (trapnr != X86_TRAP_UD)
> return 0;
>
> Copy and paste from do_error_trap() ....

As you can see, do_alignment_check() is copied from do_error_trap().
But seems this part of code is irrelevant to #AC handler.

So I will remove the "if (!user_mode(regs) && fixup_bug(regs, trapnr))"
and surrounding code, right?

>
> > + return;
> > +
> > + if (notify_die(DIE_TRAP, str, regs, error_code, trapnr, signr) ==
> > + NOTIFY_STOP)
> > + return;
> > +
> > + cond_local_irq_enable(regs);
> > + if (!user_mode(regs) &&
> > + static_cpu_has(X86_FEATURE_SPLIT_LOCK_DETECT)) {
> > + /*
> > + * Only split lock can generate #AC from kernel at this point.
> > + * Warn and disable split lock detection on this CPU. The
> > + * faulting instruction will be executed without generating
> > + * another #AC fault. User needs to check the warning and
> > + * fix the split lock issue in the faulting instruction.
>
> "User needs to check the warning and fix the issue ..."
>
> I'm looking forward to all the fixes from Joe Users.
>
> Please remove that sentence. It's useless. Users report warnings if at all
> and the kernel developers who actually look at them surely don't need an
> advice like that.

Sure. Will do this.

Thanks.

-Fenghua