[PATCH] x86/asm/entry/32: Rename labels in INT 0x80 code path

[PATCH] x86/asm/entry/32: Rename labels in INT 0x80 code path

[Denys Vlasenko]

"ia32_syscall"  label name (entry point for INT 80) is inconsistent with
"ia32_sysenter_target" (entry point for SYSENTER insn) and
"ia32_cstar_target" (entry point for SYSCALL insn).

It is especially misleading since we do have code in this file which
handles SYSCALL insn, and it's _not this code_.

Rename it to ia32_int80_target. Rename all internal labels in ia32_int80_target
routine to int80_<foo> form, similar to the convention in SYSENTER and SYSCALL
code blocks.

Not renaming ia32_cstar_target: even though "ia32_syscall_target" would make
a bit more sense, it's less greppable: "syscall" is a very common string.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/entry/entry_64_compat.S | 12 ++++++------
 arch/x86/include/asm/proto.h     |  2 +-
 arch/x86/kernel/traps.c          |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index b80f8d4..744a3f7 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -443,7 +443,7 @@ ia32_ret_from_sys_call:
  * Assumes it is only called from user space and entered with interrupts off.
  */
 
-ENTRY(ia32_syscall)
+ENTRY(ia32_int80_target)
 	/*
 	 * Interrupts are off on entry.
 	 * We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON,
@@ -472,9 +472,9 @@ ENTRY(ia32_syscall)
 
 	orl	$TS_COMPAT, ASM_THREAD_INFO(TI_status, %rsp, SIZEOF_PTREGS)
 	testl	$_TIF_WORK_SYSCALL_ENTRY, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS)
-	jnz	ia32_tracesys
+	jnz	int80_tracesys
 
-ia32_do_call:
+int80_do_call:
 	/* 32-bit syscall -> 64-bit C ABI argument conversion */
 	movl	%edi, %r8d		/* arg5 */
 	movl	%ebp, %r9d		/* arg6 */
@@ -489,7 +489,7 @@ ia32_do_call:
 1:
 	jmp	int_ret_from_sys_call
 
-ia32_tracesys:
+int80_tracesys:
 	SAVE_EXTRA_REGS
 	movq	%rsp, %rdi			/* &pt_regs -> arg1 */
 	call	syscall_trace_enter
@@ -506,8 +506,8 @@ ia32_tracesys:
 	movl	RDI(%rsp), %edi
 	movl	%eax, %eax		/* zero extension */
 	RESTORE_EXTRA_REGS
-	jmp	ia32_do_call
-END(ia32_syscall)
+	jmp	int80_do_call
+END(ia32_int80_target)
 
 	.macro PTREGSCALL label, func
 	ALIGN
diff --git a/arch/x86/include/asm/proto.h b/arch/x86/include/asm/proto.h
index a90f897..3c692be 100644
--- a/arch/x86/include/asm/proto.h
+++ b/arch/x86/include/asm/proto.h
@@ -8,7 +8,7 @@
 void system_call(void);
 void syscall_init(void);
 
-void ia32_syscall(void);
+void ia32_int80_target(void);
 void ia32_cstar_target(void);
 void ia32_sysenter_target(void);
 
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index b5e7687..b8a4204 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -894,7 +894,7 @@ void __init trap_init(void)
 		set_bit(i, used_vectors);
 
 #ifdef CONFIG_IA32_EMULATION
-	set_system_intr_gate(IA32_SYSCALL_VECTOR, ia32_syscall);
+	set_system_intr_gate(IA32_SYSCALL_VECTOR, ia32_int80_target);
 	set_bit(IA32_SYSCALL_VECTOR, used_vectors);
 #endif
 
-- 
1.8.1.4

Re: [PATCH] x86/asm/entry/32: Rename labels in INT 0x80 code path

[Ingo Molnar]

* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Jun 7, 2015 11:42 AM, "Denys Vlasenko" <dvlasenk@redhat.com> wrote:
> >
> > Rename it to ia32_int80_target.
> 
> Btw, could we arrive to get rid of the idiotic "ia32" naming too? It's wrong, 
> and it harkens back to the days when intel thought itanium makes sense and 
> wanted to talk about "intel architecture".

Absolutely, I've been slowly eliminating uses of it - that naming is very 
annoying.

Another thing I'm doing is to slowly remove references to 'emulation' - we don't 
emulate 32-bit in any way, we implement various 32-bit syscall ABIs (old a new) 
natively.

I'd like to remove CONFIG_IA32_EMULATION from the .config as well - it offers 
nothing real over CONFIG_COMPAT.

> The platform is called x86, not ia32. And in this particular case, u suspect the 
> important part isn't the x86 part, but the "compat" part - we damn well know 
> we're x86, the important part is that this is the 32-bit compat entry point of a 
> 64-bit kernel. No?

Totally. All strings of 'ia32' and 'IA32' will be gone in the long run except from 
Intel specific MSR names or so.

> So "ia32" is just crazy, and the architecture is not in question anyway, why not 
> name these things for the things that really matter?

Yeah. I wanted to rename all the entry points to be logical, and beyond removing 
the nonsensical 'ia32' names I also wanted to make it clear what kind of 
instruction's entry point they are.

For example whoever thought that 'ia32_cstar_target' is a proper name for the 
primary 32-bit syscall entry point needs their head examined.

My (re-)naming plan is:

	ia32_sysenter_target	-> entry_SYSENTER_32
	system_call (32)	-> entry_INT80_32
	system_call (64)	-> entry_SYSCALL_64
	ia32_cstar_target	-> entry_SYSCALL_compat
	ia32_syscall		-> entry_INT80_compat
	ia32_sysenter_target	-> entry_SYSENTER_compat

The ideas behind this naming scheme is to:

  - Make it really obvious, through a capitalized asm mnemonic, which particular 
    x86 CPU instruction a particular entry point corresponds to. People changing 
    that code should be absolutely aware of the various special properties these
    instructions have.

  - Harmonize the naming across the native 32-bit, 64-bit and compat space.

  - Unconfuse the 32-bit and 64-bit logic where the 'system_call' entry point is 
    actually for two (starkly) different instructions.

  - Remove the various ia32 prefixes that are sometimes denoting compat, sometimes
    native 32-bit.

Another possibility would be:

	ia32_sysenter_target	-> entry_32_SYSENTER
	system_call (32)	-> entry_32_INT80
	system_call (64)	-> entry_64_SYSCALL
	ia32_cstar_target	-> entry_64_SYSCALL_compat
	ia32_syscall		-> entry_64_INT80_compat
	ia32_sysenter_target	-> entry_64_SYSENTER_compat

These names are typically only used in two places, so name length is not as 
critical as for other function names.

And naming matters: a good name is both descriptive and short, as we know it from 
the excellent examples of 'Linux' and 'Git'. Oh wait ...

Thanks,

	Ingo

Re: [PATCH] x86/asm/entry/32: Rename labels in INT 0x80 code path

[Brian Gerst]

On Mon, Jun 8, 2015 at 1:35 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>> On Jun 7, 2015 11:42 AM, "Denys Vlasenko" <dvlasenk@redhat.com> wrote:
>> >
>> > Rename it to ia32_int80_target.
>>
>> Btw, could we arrive to get rid of the idiotic "ia32" naming too? It's wrong,
>> and it harkens back to the days when intel thought itanium makes sense and
>> wanted to talk about "intel architecture".
>
> Absolutely, I've been slowly eliminating uses of it - that naming is very
> annoying.
>
> Another thing I'm doing is to slowly remove references to 'emulation' - we don't
> emulate 32-bit in any way, we implement various 32-bit syscall ABIs (old a new)
> natively.
>
> I'd like to remove CONFIG_IA32_EMULATION from the .config as well - it offers
> nothing real over CONFIG_COMPAT.

A few months ago I started working on a set of patches to decouple the
X32 support from the 32-bit compat support.  There is actually quite a
bit of non-shared code between the two, mainly signal handling and
syscall entries.  The code used by both should be CONFIG_COMPAT, but
separate config defines should be kept for the non-shared code.  That
would allow X32 support without dragging in all of the 32-bit compat
support.  I never finished it because I couldn't find a current
distribution that supported X32 out of the box to test with.

I agree on dropping the word emulation though, since the hardware
provides a full 32-bit environment.  I suggest CONFIG_X86_32_COMPAT
(or CONFIG_COMPAT_X86_32) as a new name.

--
Brian Gerst

Re: [PATCH] x86/asm/entry/32: Rename labels in INT 0x80 code path

[Ingo Molnar]

* Brian Gerst <brgerst@gmail.com> wrote:

> On Mon, Jun 8, 2015 at 1:35 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> >> On Jun 7, 2015 11:42 AM, "Denys Vlasenko" <dvlasenk@redhat.com> wrote:
> >> >
> >> > Rename it to ia32_int80_target.
> >>
> >> Btw, could we arrive to get rid of the idiotic "ia32" naming too? It's wrong, 
> >> and it harkens back to the days when intel thought itanium makes sense and 
> >> wanted to talk about "intel architecture".
> >
> > Absolutely, I've been slowly eliminating uses of it - that naming is very 
> > annoying.
> >
> > Another thing I'm doing is to slowly remove references to 'emulation' - we 
> > don't emulate 32-bit in any way, we implement various 32-bit syscall ABIs (old 
> > a new) natively.
> >
> > I'd like to remove CONFIG_IA32_EMULATION from the .config as well - it offers 
> > nothing real over CONFIG_COMPAT.
> 
> A few months ago I started working on a set of patches to decouple the X32 
> support from the 32-bit compat support.  There is actually quite a bit of 
> non-shared code between the two, mainly signal handling and syscall entries.  
> The code used by both should be CONFIG_COMPAT, but separate config defines 
> should be kept for the non-shared code.  That would allow X32 support without 
> dragging in all of the 32-bit compat support.  I never finished it because I 
> couldn't find a current distribution that supported X32 out of the box to test 
> with.

So yes, x32 support outlined some of the shortcomings of our current compat code, 
which pretty much assumes that there's just a single compat model - which is far 
away from reality on x86, which has 3 compat modes:

 - 16-bit registers, 16-bit memory model (vm86 mode)
 - 32-bit registers, 32-bit memory model (CONFIG_COMPAT)
 - 64-bit registers, 32-bit memory model (CONFIG_X86_X32_ABI)

and both the naming and (necessarily) the organization of the code is suffering a 
bit from that currently.

Perhaps we could improve things by reorganizing it along 'multiple ABIs' idiom, 
which x32 partially already started.

The most important aspect of 'compat', in terms of complexity, is pointer size, 
i.e. the memory model. Register width of the ABI matters too, but is mostly 
resolved early on during system call entry. Pointer size details matter all across 
the kernel, in system calls that interact via user-space pointers.

> I agree on dropping the word emulation though, since the hardware provides a 
> full 32-bit environment.  I suggest CONFIG_X86_32_COMPAT (or 
> CONFIG_COMPAT_X86_32) as a new name.

So why not just use CONFIG_COMPAT and allow the configuration of the 3 system call 
ABIs:

 - CONFIG_SYSCALL_ABI_X32
 - CONFIG_SYSCALL_ABI_32
 - CONFIG_SYSCALL_ABI_64

Where 64-bit is always enabled on 64-bit kernels and ABI_32 is always enabled on 
32-bit kernels.

ABI_X32 enables the extra system calls that have 64-bit register width but a 
compact 32-bit memory model.

( I don't think we want CONFIG_ABI_16, because vm86 is really special, it's 
  essentially an early x86 hypervisor implementation with no separate system call
  ABI. )

CONFIG_IA32_EMULATION is not just a double misnomer (it's not Intel specific and 
it's not emulation), it also does almost nothing useful, all it does is to enable 
a few options:

        select BINFMT_ELF
        select COMPAT_BINFMT_ELF
        select HAVE_UID16

and offers a legacy (aout) binformat.

Note that the selects are wrong already: for example there's absolutely nothing 
wrong about an x32 environment without legacy HAVE_UID16 support.

Instead of that we should simply introduce CONFIG_SYSCALL_ABI_* on x86 and 
implicitly select CONFIG_COMPAT, which is the generic kernel's 32-bit memory 
pointer model selector. (Question: do we have any architecture where compat isn't 
32-bit with native being 64-bit?)

Hm?

Thanks,

	Ingo

Re: [PATCH] x86/asm/entry/32: Rename labels in INT 0x80 code path

[Andy Lutomirski]

On Mon, Jun 8, 2015 at 12:14 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Brian Gerst <brgerst@gmail.com> wrote:
>
>> On Mon, Jun 8, 2015 at 1:35 AM, Ingo Molnar <mingo@kernel.org> wrote:
>> >
>> > * Linus Torvalds <torvalds@linux-foundation.org> wrote:
>> >
>> >> On Jun 7, 2015 11:42 AM, "Denys Vlasenko" <dvlasenk@redhat.com> wrote:
>> >> >
>> >> > Rename it to ia32_int80_target.
>> >>
>> >> Btw, could we arrive to get rid of the idiotic "ia32" naming too? It's wrong,
>> >> and it harkens back to the days when intel thought itanium makes sense and
>> >> wanted to talk about "intel architecture".
>> >
>> > Absolutely, I've been slowly eliminating uses of it - that naming is very
>> > annoying.
>> >
>> > Another thing I'm doing is to slowly remove references to 'emulation' - we
>> > don't emulate 32-bit in any way, we implement various 32-bit syscall ABIs (old
>> > a new) natively.
>> >
>> > I'd like to remove CONFIG_IA32_EMULATION from the .config as well - it offers
>> > nothing real over CONFIG_COMPAT.
>>
>> A few months ago I started working on a set of patches to decouple the X32
>> support from the 32-bit compat support.  There is actually quite a bit of
>> non-shared code between the two, mainly signal handling and syscall entries.
>> The code used by both should be CONFIG_COMPAT, but separate config defines
>> should be kept for the non-shared code.  That would allow X32 support without
>> dragging in all of the 32-bit compat support.  I never finished it because I
>> couldn't find a current distribution that supported X32 out of the box to test
>> with.
>
> So yes, x32 support outlined some of the shortcomings of our current compat code,
> which pretty much assumes that there's just a single compat model - which is far
> away from reality on x86, which has 3 compat modes:
>
>  - 16-bit registers, 16-bit memory model (vm86 mode)
>  - 32-bit registers, 32-bit memory model (CONFIG_COMPAT)
>  - 64-bit registers, 32-bit memory model (CONFIG_X86_X32_ABI)
>
> and both the naming and (necessarily) the organization of the code is suffering a
> bit from that currently.
>
> Perhaps we could improve things by reorganizing it along 'multiple ABIs' idiom,
> which x32 partially already started.
>
> The most important aspect of 'compat', in terms of complexity, is pointer size,
> i.e. the memory model. Register width of the ABI matters too, but is mostly
> resolved early on during system call entry. Pointer size details matter all across
> the kernel, in system calls that interact via user-space pointers.
>
>> I agree on dropping the word emulation though, since the hardware provides a
>> full 32-bit environment.  I suggest CONFIG_X86_32_COMPAT (or
>> CONFIG_COMPAT_X86_32) as a new name.
>
> So why not just use CONFIG_COMPAT and allow the configuration of the 3 system call
> ABIs:
>
>  - CONFIG_SYSCALL_ABI_X32
>  - CONFIG_SYSCALL_ABI_32

This would be quite nice -- we could get rid off all the
defined(CONFIG_X86_32) || defined(IA32_EMULATION) checks.

>  - CONFIG_SYSCALL_ABI_64
>
> Where 64-bit is always enabled on 64-bit kernels and ABI_32 is always enabled on
> 32-bit kernels.
>
> ABI_X32 enables the extra system calls that have 64-bit register width but a
> compact 32-bit memory model.
>
> ( I don't think we want CONFIG_ABI_16, because vm86 is really special, it's
>   essentially an early x86 hypervisor implementation with no separate system call
>   ABI. )

Agreed.

--Andy