From: Andy Lutomirski <luto@kernel.org>
To: Andrew Lutomirski <luto@kernel.org>
Cc: "Bae, Chang Seok" <chang.seok.bae@intel.com>,
Ingo Molnar <mingo@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
"H. Peter Anvin" <hpa@zytor.com>, Andi Kleen <ak@linux.intel.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
"Metzger, Markus T" <markus.t.metzger@intel.com>,
"Ravi V. Shankar" <ravi.v.shankar@intel.com>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [regression in -rc1] Re: [PATCH v6 2/8] x86/fsgsbase/64: Introduce FS/GS base helper functions
Date: Thu, 25 Oct 2018 15:37:19 -0700 [thread overview]
Message-ID: <CALCETrUGqfpcu6gf2931f9kfkzirTCojANt3BncTY7h9usQYtA@mail.gmail.com> (raw)
In-Reply-To: <CALCETrUr3DGwW8y6y_cffEKWO_KQs1J2_UvTBZsbdAzES_f-Ag@mail.gmail.com>
On Wed, Oct 24, 2018 at 12:01 PM Andy Lutomirski <luto@kernel.org> wrote:
>
> On Tue, Sep 18, 2018 at 4:09 PM Chang S. Bae <chang.seok.bae@intel.com> wrote:
> >
> > With new helpers, FS/GS base access is centralized.
> > Eventually, when FSGSBASE instruction enabled, it will
> > be faster.
>
> Sorry for not catching this during review, but:
>
> > +void x86_fsbase_write_cpu(unsigned long fsbase)
> > +{
> > + /*
> > + * Set the selector to 0 as a notion, that the segment base is
> > + * overwritten, which will be checked for skipping the segment load
> > + * during context switch.
> > + */
> > + loadseg(FS, 0);
>
> ^^^
>
> what?
>
> > + wrmsrl(MSR_FS_BASE, fsbase);
> > +}
>
> I don't understand what the comment is trying to say, but the sole
> caller so far of this function is x86_gsbase_write_task(), and the
> code looks incorrect.
>
> Ingo, I think we need to address this during this merge window,
> probably by removing the comment and the loadseg() call (and the same
> for gsbase...inactive). But first, Chang, can you explain what
> exactly your intent is here?
It might not be a problem for the current merge window, since the one
and only caller (I think) that hits this code is okay with it. But it
might still be nice to have it cleaned up in Linus' tree.
next prev parent reply other threads:[~2018-10-25 22:37 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-18 23:08 [PATCH v6 0/8] x86: infrastructure to enable FSGSBASE Chang S. Bae
2018-09-18 23:08 ` [PATCH v6 1/8] x86/arch_prctl/64: Make ptrace read FS/GS base accurately Chang S. Bae
2018-10-08 9:54 ` [tip:x86/asm] x86/fsgsbase/64: Fix ptrace() to read the " tip-bot for Andy Lutomirski
2018-10-08 9:59 ` [tip:x86/asm] x86/segments: Introduce the 'CPUNODE' naming to better document the segment limit CPU/node NR trick tip-bot for Ingo Molnar
2018-10-08 9:59 ` [tip:x86/asm] x86/fsgsbase/64: Clean up various details tip-bot for Ingo Molnar
2018-09-18 23:08 ` [PATCH v6 2/8] x86/fsgsbase/64: Introduce FS/GS base helper functions Chang S. Bae
2018-10-08 9:55 ` [tip:x86/asm] " tip-bot for Chang S. Bae
2018-10-24 19:01 ` [regression in -rc1] Re: [PATCH v6 2/8] " Andy Lutomirski
2018-10-24 19:13 ` Bae, Chang Seok
2018-10-24 19:22 ` Andy Lutomirski
2018-10-24 19:29 ` Bae, Chang Seok
2018-10-24 19:43 ` Andy Lutomirski
2018-10-24 22:50 ` Bae, Chang Seok
2018-10-25 22:37 ` Andy Lutomirski [this message]
2018-09-18 23:08 ` [PATCH v6 3/8] x86/fsgsbase/64: Make ptrace use correct FS/GS base helpers Chang S. Bae
2018-10-08 9:56 ` [tip:x86/asm] x86/fsgsbase/64: Make ptrace use the new " tip-bot for Chang S. Bae
2018-09-18 23:08 ` [PATCH v6 4/8] x86/fsgsbase/64: Use FS/GS base helpers in core dump Chang S. Bae
2018-10-08 9:56 ` [tip:x86/asm] x86/fsgsbase/64: Convert the ELF core dump code to the new FSGSBASE helpers tip-bot for Chang S. Bae
2018-09-18 23:08 ` [PATCH v6 5/8] x86/fsgsbase/64: Factor out load FS/GS segments from __switch_to() Chang S. Bae
2018-10-08 9:57 ` [tip:x86/asm] x86/fsgsbase/64: Factor out FS/GS segment loading " tip-bot for Chang S. Bae
2018-09-18 23:08 ` [PATCH v6 6/8] x86/segments/64: Rename PER_CPU segment to CPU_NUMBER Chang S. Bae
2018-10-08 9:57 ` [tip:x86/asm] x86/segments/64: Rename the GDT PER_CPU entry " tip-bot for Chang S. Bae
2018-09-18 23:08 ` [PATCH v6 7/8] x86/vdso: Introduce helper functions for CPU and node number Chang S. Bae
2018-10-08 9:58 ` [tip:x86/asm] " tip-bot for Chang S. Bae
2018-09-18 23:08 ` [PATCH v6 8/8] x86/vdso: Move out the CPU initialization Chang S. Bae
2018-10-08 8:36 ` Ingo Molnar
2018-10-08 9:58 ` [tip:x86/asm] x86/vdso: Initialize the CPU/node NR segment descriptor earlier tip-bot for Chang S. Bae
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALCETrUGqfpcu6gf2931f9kfkzirTCojANt3BncTY7h9usQYtA@mail.gmail.com \
--to=luto@kernel.org \
--cc=ak@linux.intel.com \
--cc=chang.seok.bae@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=markus.t.metzger@intel.com \
--cc=mingo@kernel.org \
--cc=ravi.v.shankar@intel.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).