linux-kernel - Re: [PATCH 1/2] x86/head_64.S: Rename start

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 7 Mar 2017 10:06:38 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     Borislav Petkov <bp@...en8.de>
Cc:     X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 1/2] x86/head_64.S: Rename start_cpu()


* Borislav Petkov <bp@...en8.de> wrote:

> From: Borislav Petkov <bp@...e.de>
> 
> It doesn't really start a CPU but does a far jump to C code. So call it
> that. Eliminate the unconditional JMP to it from secondary_startup_64()
> but make the jump to C code piece part of secondary_startup_64()
> instead.
> 
> Also, it doesn't need to be a global symbol either so make it a local
> label. One less needlessly global symbol in the symbol table.
> 
> No functionality change.
> 
> Signed-off-by: Borislav Petkov <bp@...e.de>
> ---
>  arch/x86/kernel/head_64.S | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> index b467b14b03eb..ac9d327d2e42 100644
> --- a/arch/x86/kernel/head_64.S
> +++ b/arch/x86/kernel/head_64.S
> @@ -269,10 +269,8 @@ ENTRY(secondary_startup_64)
>  	/* rsi is pointer to real mode structure with interesting info.
>  	   pass it to C */
>  	movq	%rsi, %rdi
> -	jmp	start_cpu
> -ENDPROC(secondary_startup_64)
>  
> -ENTRY(start_cpu)
> +.Ljump_to_C_code:
>  	/*
>  	 * Jump to run C code and to be on a real kernel address.
>  	 * Since we are running on identity-mapped space we have to jump
> @@ -305,7 +303,7 @@ ENTRY(start_cpu)
>  	pushq	%rax		# target address in negative space
>  	lretq
>  .Lafter_lret:
> -ENDPROC(start_cpu)
> +ENDPROC(secondary_startup_64)
>  
>  #include "verify_cpu.S"
>  
> @@ -313,11 +311,11 @@ ENDPROC(start_cpu)
>  /*
>   * Boot CPU0 entry point. It's called from play_dead(). Everything has been set
>   * up already except stack. We just set up stack here. Then call
> - * start_secondary() via start_cpu().
> + * start_secondary() via .Ljump_to_C_code.
>   */
>  ENTRY(start_cpu0)
>  	movq	initial_stack(%rip), %rsp
> -	jmp	start_cpu
> +	jmp	.Ljump_to_C_code
>  ENDPROC(start_cpu0)
>  #endif

Wouldn't this be slightly more readable:

	jmp	.L_jump_to_C_code

? (Note the extra underscore in the symbol name)

The local labels syntax is silly, I always end up looking twice to make sense of 
'Ljump' or 'Lwhatever' ;-)

We cannot do anything about that - but we can name our symbols to work it around. 
But the price is the extra underscore in the symbol name...

Thanks,

	Ingo