Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1191163 > unrolled thread

Re: [PATCH 1/5] x86/entry/32: Clean up enable_sep_cpu to prepare for 64-bit merge

Started byBrian Gerst <brgerst@gmail.com>
First post2015-07-23 19:30 +0200
Last post2015-07-23 21:00 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 1/5] x86/entry/32: Clean up enable_sep_cpu to prepare for  64-bit merge Brian Gerst <brgerst@gmail.com> - 2015-07-23 19:30 +0200
    Re: [PATCH 1/5] x86/entry/32: Clean up enable_sep_cpu to prepare for  64-bit merge Andy Lutomirski <luto@amacapital.net> - 2015-07-23 20:10 +0200
      Re: [PATCH 1/5] x86/entry/32: Clean up enable_sep_cpu to prepare for  64-bit merge Andy Lutomirski <luto@amacapital.net> - 2015-07-23 20:40 +0200
        Re: [PATCH 1/5] x86/entry/32: Clean up enable_sep_cpu to prepare for  64-bit merge Andy Lutomirski <luto@amacapital.net> - 2015-07-23 20:50 +0200
          Re: [PATCH 1/5] x86/entry/32: Clean up enable_sep_cpu to prepare for  64-bit merge Linus Torvalds <torvalds@linux-foundation.org> - 2015-07-23 21:00 +0200

#1191163 — Re: [PATCH 1/5] x86/entry/32: Clean up enable_sep_cpu to prepare for 64-bit merge

FromBrian Gerst <brgerst@gmail.com>
Date2015-07-23 19:30 +0200
SubjectRe: [PATCH 1/5] x86/entry/32: Clean up enable_sep_cpu to prepare for 64-bit merge
Message-ID<pPstJ-4VP-31@gated-at.bofh.it>
On Thu, Jul 23, 2015 at 12:02 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Jul 23, 2015 at 8:56 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> On Thu, 23 Jul 2015 08:31:39 -0700
>> Andy Lutomirski <luto@kernel.org> wrote:
>>
>>> Switch from wrmsr to wrmsrl_safe to prepare to merge the 32-bit and
>>> 64-bit code, and use __KERNEL_CS explicitly to initialize
>>> MSR_IA32_SYSENTER_CS.  While we're at it, tweak the whitespace a
>>> bit.
>>
>> Saying "prepare to merge the 32-bit and 64-bit" isn't that informative
>> to why this patch is needed. Can you please describe in the change log
>> what can fault with the wrmsr call, when we do the merge?
>
> Oh...
>
> Nothing can fault in the wrmsr as far as I know, but maybe there's a
> CPU out there that advertises SEP but doesn't have the MSRs.
>
> Ingo, Thomas, want a v2 that explains that the wrmsr_safe is there for
> consistency but that I don't know why 32bit does it?  Another option
> would be to remove the _safe from the 32-bit version.

There is an erratum with SEP being advertised but not available on
early P6 models, but we already check for that in the cpu setup code.
There shouldn't be a problem with a fault.

--
Brian Gerst
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1191192

FromAndy Lutomirski <luto@amacapital.net>
Date2015-07-23 20:10 +0200
Message-ID<pPt6q-5Um-27@gated-at.bofh.it>
In reply to#1191163
On Thu, Jul 23, 2015 at 10:27 AM, Brian Gerst <brgerst@gmail.com> wrote:
> On Thu, Jul 23, 2015 at 12:02 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Thu, Jul 23, 2015 at 8:56 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>>> On Thu, 23 Jul 2015 08:31:39 -0700
>>> Andy Lutomirski <luto@kernel.org> wrote:
>>>
>>>> Switch from wrmsr to wrmsrl_safe to prepare to merge the 32-bit and
>>>> 64-bit code, and use __KERNEL_CS explicitly to initialize
>>>> MSR_IA32_SYSENTER_CS.  While we're at it, tweak the whitespace a
>>>> bit.
>>>
>>> Saying "prepare to merge the 32-bit and 64-bit" isn't that informative
>>> to why this patch is needed. Can you please describe in the change log
>>> what can fault with the wrmsr call, when we do the merge?
>>
>> Oh...
>>
>> Nothing can fault in the wrmsr as far as I know, but maybe there's a
>> CPU out there that advertises SEP but doesn't have the MSRs.
>>
>> Ingo, Thomas, want a v2 that explains that the wrmsr_safe is there for
>> consistency but that I don't know why 32bit does it?  Another option
>> would be to remove the _safe from the 32-bit version.
>
> There is an erratum with SEP being advertised but not available on
> early P6 models, but we already check for that in the cpu setup code.
> There shouldn't be a problem with a fault.

I can make a v2 that gets rid of the _safe.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1191212

FromAndy Lutomirski <luto@amacapital.net>
Date2015-07-23 20:40 +0200
Message-ID<pPtzr-6sr-9@gated-at.bofh.it>
In reply to#1191192
On Thu, Jul 23, 2015 at 11:01 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Jul 23, 2015 at 10:27 AM, Brian Gerst <brgerst@gmail.com> wrote:
>> On Thu, Jul 23, 2015 at 12:02 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Thu, Jul 23, 2015 at 8:56 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>>>> On Thu, 23 Jul 2015 08:31:39 -0700
>>>> Andy Lutomirski <luto@kernel.org> wrote:
>>>>
>>>>> Switch from wrmsr to wrmsrl_safe to prepare to merge the 32-bit and
>>>>> 64-bit code, and use __KERNEL_CS explicitly to initialize
>>>>> MSR_IA32_SYSENTER_CS.  While we're at it, tweak the whitespace a
>>>>> bit.
>>>>
>>>> Saying "prepare to merge the 32-bit and 64-bit" isn't that informative
>>>> to why this patch is needed. Can you please describe in the change log
>>>> what can fault with the wrmsr call, when we do the merge?
>>>
>>> Oh...
>>>
>>> Nothing can fault in the wrmsr as far as I know, but maybe there's a
>>> CPU out there that advertises SEP but doesn't have the MSRs.
>>>
>>> Ingo, Thomas, want a v2 that explains that the wrmsr_safe is there for
>>> consistency but that I don't know why 32bit does it?  Another option
>>> would be to remove the _safe from the 32-bit version.
>>
>> There is an erratum with SEP being advertised but not available on
>> early P6 models, but we already check for that in the cpu setup code.
>> There shouldn't be a problem with a fault.
>
> I can make a v2 that gets rid of the _safe.
>

Ugh.  wrmsrl_safe is a function but wrmsrl isn't.  I'll fix that and send a v2.

Gah, macros.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1191223

FromAndy Lutomirski <luto@amacapital.net>
Date2015-07-23 20:50 +0200
Message-ID<pPtJ8-6DM-19@gated-at.bofh.it>
In reply to#1191212
On Thu, Jul 23, 2015 at 11:33 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Jul 23, 2015 at 11:01 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Thu, Jul 23, 2015 at 10:27 AM, Brian Gerst <brgerst@gmail.com> wrote:
>>> On Thu, Jul 23, 2015 at 12:02 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> On Thu, Jul 23, 2015 at 8:56 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>>>>> On Thu, 23 Jul 2015 08:31:39 -0700
>>>>> Andy Lutomirski <luto@kernel.org> wrote:
>>>>>
>>>>>> Switch from wrmsr to wrmsrl_safe to prepare to merge the 32-bit and
>>>>>> 64-bit code, and use __KERNEL_CS explicitly to initialize
>>>>>> MSR_IA32_SYSENTER_CS.  While we're at it, tweak the whitespace a
>>>>>> bit.
>>>>>
>>>>> Saying "prepare to merge the 32-bit and 64-bit" isn't that informative
>>>>> to why this patch is needed. Can you please describe in the change log
>>>>> what can fault with the wrmsr call, when we do the merge?
>>>>
>>>> Oh...
>>>>
>>>> Nothing can fault in the wrmsr as far as I know, but maybe there's a
>>>> CPU out there that advertises SEP but doesn't have the MSRs.
>>>>
>>>> Ingo, Thomas, want a v2 that explains that the wrmsr_safe is there for
>>>> consistency but that I don't know why 32bit does it?  Another option
>>>> would be to remove the _safe from the 32-bit version.
>>>
>>> There is an erratum with SEP being advertised but not available on
>>> early P6 models, but we already check for that in the cpu setup code.
>>> There shouldn't be a problem with a fault.
>>
>> I can make a v2 that gets rid of the _safe.
>>
>
> Ugh.  wrmsrl_safe is a function but wrmsrl isn't.  I'll fix that and send a v2.
>
> Gah, macros.

On third thought, no v2.  It's *64-bit* CPUs that might want the
_safe.  We aren't currently checking SEP.  Maybe there's a non-SEP AMD
CPU and we never noticed because SEP was never useful on AMD 64-bit
CPUs.

I'll send a followup.

>
> --Andy



-- 
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1191225

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2015-07-23 21:00 +0200
Message-ID<pPtSO-6Pc-21@gated-at.bofh.it>
In reply to#1191223
On Thu, Jul 23, 2015 at 11:42 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>
> On third thought, no v2.  It's *64-bit* CPUs that might want the
> _safe.  We aren't currently checking SEP.  Maybe there's a non-SEP AMD
> CPU and we never noticed because SEP was never useful on AMD 64-bit
> CPUs.

I don't see the downside of using the safe version unconditionally. No
need to worry about any corner cases.

                  Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web