lkml.org 
[lkml]   [2015]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 2/4] x86/asm/entry: Untangle 'ia32_sysenter_target' into two entry points: entry_SYSENTER_32 and entry_SYSENTER_compat
On Tue, Jun 9, 2015 at 2:33 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Andy Lutomirski <luto@amacapital.net> wrote:
>
>> On Mon, Jun 8, 2015 at 1:34 AM, Ingo Molnar <mingo@kernel.org> wrote:
>> > So the SYSENTER instruction is pretty quirky and it has different behavior
>> > depending on bitness and CPU maker.
>> >
>> > Yet we create a false sense of coherency by naming it 'ia32_sysenter_target'
>> > in both of the cases.
>> >
>> > Split the name into its two uses:
>> >
>> > ia32_sysenter_target (32) -> entry_SYSENTER_32
>> > ia32_sysenter_target (64) -> entry_SYSENTER_compat
>> >
>>
>> Now that I'm rebasing my pile on top of this, I have a minor gripe
>> about this one. There are (in my mind, anyway), two SYSENTER
>> instructions: the 32-bit one and the 64-bit one. (That is, there's
>> SYSENTER32, which happens when you do SYSENTER in 32-bit or compat
>> mode, and SYSENTER64, which happens when you do SYSENTER in long
>> mode.) SYSENTER32, from user code's perspective, does the same thing
>> in either case [1]. That means that it really does make sense that
>> we'd have two implementations of the same entry point, one written in
>> 32-bit asm and one written in 64-bit asm.
>>
>> The patch I'm rebasing merges the two wrmsrs to MSR_IA32_SYSENTER, and
>> this change makes it uglier.
>>
>> [1] Sort of. We probably have differently nonsensical calling
>> conventions, but that's our fault and has nothing to do with the
>> hardware.
>
> Did you intend to merge these two wrmsr()s:
>
> #ifdef CONFIG_X86_64
> void syscall_init(void)
> {
> ...
> wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)entry_SYSENTER_compat);
> ...
> }
> #endif
>
> #ifdef CONFIG_X86_32
> void enable_sep_cpu(void)
> {
> ...
> wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long)entry_SYSENTER_32, 0);
> ...
> }
>
> ... and the new bifurcated names preserve the #ifdef, right?

Exactly.

>
> So I mostly agree with you, but still I'm a bit torn about this, for the following
> reason:
>
> - SYSENTER on a 32-bit kernel behaves a bit differently from SYSENTER on a 64-bit
> kernel: for example on 32-bit kernels we'll return with SYSEXIT, while on
> 64-bit kernels we return with SYSRET. The difference is small but user-space
> observable: for example EDX is 0 on SYSRET while it points to ->sysenter_return
> in the SYSEXIT case.
>
> This kind of user-observable assymmetry does not exist for other unified syscall
> ABIs, such as the INT80 method.
>
> So I think that despite having to preserve a small non-unified #ifdef for this
> initialization, we are still better off naming the two entry points differently,
> along the pattern we use, because the behavior is slightly different depending on
> the bitness of the kernel.
>

Fair enough. This is certainly not a big deal either way. Maybe when
this really gets cleaned up, we can merge the entry points again.

--Andy

> Thanks,
>
> Ingo



--
Andy Lutomirski
AMA Capital Management, LLC


\
 
 \ /
  Last update: 2015-06-09 19:01    [W:0.060 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site