From: hpa@zytor.com
To: Brian Gerst <brgerst@gmail.com>, Andy Lutomirski <luto@kernel.org>
Cc: "Bae, Chang Seok" <chang.seok.bae@intel.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@kernel.org>, 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: [PATCH v2 7/8] x86/segments/32: Introduce CPU_NUMBER segment
Date: Wed, 06 Jun 2018 15:53:02 -0700 [thread overview]
Message-ID: <F261317E-688B-4638-AB58-75355D97FD6C@zytor.com> (raw)
In-Reply-To: <CAMzpN2iE1zba6g6Jh5LzzuX1AM+SskiwVtVi8WtcB+4dD2mJGQ@mail.gmail.com>
On June 6, 2018 12:07:15 PM PDT, Brian Gerst <brgerst@gmail.com> wrote:
>On Wed, Jun 6, 2018 at 1:16 PM, Andy Lutomirski <luto@kernel.org>
>wrote:
>> On Wed, Jun 6, 2018 at 9:23 AM Chang S. Bae
><chang.seok.bae@intel.com> wrote:
>>>
>>> The new entry will be equivalent to that of x86-64 which
>>> stores CPU number. The entry is placed in segment 23 in GDT
>>> by bumping down 23-28 by one, which are all kernel-internal
>>> segments and so have no impact on user space.
>>>
>>> CPU_NUMBER segment will always be at '%ss (USER_DS) + 80'
>>> for the default (flat, initial) user space %ss.
>>
>> No, it won't :( This is because, on Xen PV, user code very
>frequently
>> sees a different, Xen-supplied "flat" SS value. This is definitely
>> true right now on 64-bit, and I'm reasonably confident it's also the
>> case on 32-bit.
>>
>> As it stands, as far as I can tell, we don't have a "cpu number"
>> segment on 32-bit kernels. I see no compelling reason to add one,
>and
>> we should definitely not add one as part of the FSGSBASE series. I
>> think the right solution is to rename the 64-bit segment to
>> "CPU_NUMBER" and then have the rearrangement of the initialization
>> code as a followup patch. The goal is to make the patches
>> individually reviewable. As it stands, this patch adds some #defines
>> without making them work, which is extra confusing.
>>
>> Given how many times we screwed it up, I really want the patch that
>> moves the initialization of the 64-bit CPU number to be obviously
>> correct and to avoid changing the sematics of anything except the
>> actual CPU number fields during boot.
>>
>> So NAK to this patch, at least as part of the FSGSBASE series.
>>
>> (My apologies -- a bunch of this is because I along with everyone
>else
>> misunderstood the existing code.)
>
>The sole purpose of this segment is for the getcpu() function in the
>VDSO. No other userspace code can rely on its presence or location.
>
>--
>Brian Gerst
Unfortunately that is not true in reality :(
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
next prev parent reply other threads:[~2018-06-06 22:53 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-06 16:23 [PATCH v2 0/8] x86: infrastructure to enable FSGSBASE Chang S. Bae
2018-06-06 16:23 ` [PATCH v2 1/8] x86/fsgsbase/64: Introduce FS/GS base helper functions Chang S. Bae
2018-06-06 16:23 ` [PATCH v2 2/8] x86/fsgsbase/64: Make ptrace read FS/GS base accurately Chang S. Bae
2018-06-06 16:23 ` [PATCH v2 3/8] x86/fsgsbase/64: Use FS/GS base helpers in core dump Chang S. Bae
2018-06-06 16:23 ` [PATCH v2 4/8] x86/fsgsbase/64: Factor out load FS/GS segments from __switch_to Chang S. Bae
2018-06-06 16:23 ` [PATCH v2 5/8] x86/msr: write_rdtscp_aux() to use wrmsr_safe() Chang S. Bae
2018-06-06 17:11 ` Andy Lutomirski
2018-06-06 16:23 ` [PATCH v2 6/8] x86/segments/64: Rename PER_CPU segment to CPU_NUMBER Chang S. Bae
2018-06-06 17:16 ` Andy Lutomirski
2018-06-06 16:23 ` [PATCH v2 7/8] x86/segments/32: Introduce CPU_NUMBER segment Chang S. Bae
2018-06-06 17:16 ` Andy Lutomirski
2018-06-06 19:07 ` Brian Gerst
2018-06-06 19:24 ` Andy Lutomirski
2018-06-06 22:53 ` hpa [this message]
2018-06-06 16:23 ` [PATCH v2 8/8] x86/vdso: Move out the CPU number store Chang S. Bae
2018-06-06 17:25 ` Andy Lutomirski
2018-06-06 18:27 ` Bae, Chang Seok
2018-06-06 22:59 ` Bae, Chang Seok
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=F261317E-688B-4638-AB58-75355D97FD6C@zytor.com \
--to=hpa@zytor.com \
--cc=ak@linux.intel.com \
--cc=brgerst@gmail.com \
--cc=chang.seok.bae@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@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
all inboxes | Powered by JetHome®