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;
MIME-Version: 1.0
References: <20181205163624.1842-1-sean.j.christopherson@intel.com>
 <20181206073409.GA64887@gmail.com> <CAHk-=wgrpUQyux2ZTJj8FUEdHcEoM0KUNrN3KCc+bBhi0fRhaQ@mail.gmail.com>
 <CALCETrUc7Y+WFkS1=ECXgtLoUXb3HPXsF+x+TCpiuuh3sXFjfA@mail.gmail.com> <CAHk-=wgOT0DW8a+VNgmrVybY1RJCajjJKskktzfNd1iJfhuV5Q@mail.gmail.com>
In-Reply-To: <CAHk-=wgOT0DW8a+VNgmrVybY1RJCajjJKskktzfNd1iJfhuV5Q@mail.gmail.com>
From:   Andy Lutomirski <luto@kernel.org>
Date:   Thu, 6 Dec 2018 12:28:16 -0800
Message-ID: <CALCETrWNGgYtFDQCBOH77dec3RctjsrMQaDuYaA4Ym5Zdo5SVQ@mail.gmail.com>
Subject: Re: [PATCH] x86/mm/fault: Streamline the fault error_code decoder
 some more
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Andrew Lutomirski <luto@kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        "Christopherson, Sean J" <sean.j.christopherson@intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        X86 ML <x86@kernel.org>, "H. Peter Anvin" <hpa@zytor.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Rik van Riel <riel@surriel.com>,
        Yu-cheng Yu <yu-cheng.yu@intel.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Dec 6, 2018 at 12:24 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Dec 6, 2018 at 11:07 AM Andy Lutomirski <luto@kernel.org> wrote:
> >
> > How do you like the attached patch?
>
> I agree with whoever thought it's odd that "read" is in lower case
> when everything else is in upper case.

"read" isn't an actual bit in the error code, so I thought it would be
polite to make it look a little bit different.  That's not a big deal,
though.

>
> And honestly, I'd just siggest making the err_text simply have the
> real user/kernel difference in it too, using something like
>
>         pr_alert("#PF error: 0x%04lx%s from %s mode\n", error_code, err_txt,
>                 user_mode(regs) ? "user" : "kernel" );
>
> The "oh, PF_USER and user_mode differs, so let's point that out
> explicitly" thing is a good thing regardless.

Sure.  Although it's extremely odd for us to OOPS from user mode, so
maybe the OOPS code in general should print a big fat warning, and
we'll just otherwise assume it was from kernel mode.  (In your tree
right now, we print the wrong value for CS for OOPSes from user mode,
which is extra confusing.  That's fixed in -tip.)

--Andy