* [PATCH 1/2] x86/head_64.S: Rename start_cpu()
@ 2017-03-04 9:56 Borislav Petkov
2017-03-04 9:56 ` [PATCH 2/2] x86/head_64.S: Pass struct boot_params' virtual address to C Borislav Petkov
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Borislav Petkov @ 2017-03-04 9:56 UTC (permalink / raw)
To: X86 ML; +Cc: LKML
From: Borislav Petkov <bp@suse.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@suse.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
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] x86/head_64.S: Pass struct boot_params' virtual address to C
2017-03-04 9:56 [PATCH 1/2] x86/head_64.S: Rename start_cpu() Borislav Petkov
@ 2017-03-04 9:56 ` Borislav Petkov
2017-03-07 9:11 ` Ingo Molnar
2017-03-07 9:06 ` [PATCH 1/2] x86/head_64.S: Rename start_cpu() Ingo Molnar
2017-03-07 13:01 ` [tip:x86/boot] x86/boot/64: " tip-bot for Borislav Petkov
2 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2017-03-04 9:56 UTC (permalink / raw)
To: X86 ML; +Cc: LKML
From: Borislav Petkov <bp@suse.de>
... so that callees don't have to convert it and can use it directly.
Simplifies C code a bit. Cleanup comments and formatting in the
vicinity, while at it.
Also, document what phys_base really is.
No functionality change.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/kernel/head64.c | 8 ++++----
arch/x86/kernel/head_64.S | 10 +++++++---
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 54a2372f5dbb..17ee20a9b4e4 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -118,7 +118,7 @@ static unsigned long get_cmd_line_ptr(void)
static void __init copy_bootdata(char *real_mode_data)
{
- char * command_line;
+ char *command_line;
unsigned long cmd_line_ptr;
memcpy(&boot_params, real_mode_data, sizeof boot_params);
@@ -130,7 +130,7 @@ static void __init copy_bootdata(char *real_mode_data)
}
}
-asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
+asmlinkage __visible void __init x86_64_start_kernel(char *real_mode_data)
{
int i;
@@ -163,7 +163,7 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
set_intr_gate(i, early_idt_handler_array[i]);
load_idt((const struct desc_ptr *)&idt_descr);
- copy_bootdata(__va(real_mode_data));
+ copy_bootdata(real_mode_data);
/*
* Load microcode early on BSP.
@@ -180,7 +180,7 @@ void __init x86_64_start_reservations(char *real_mode_data)
{
/* version is always not zero if it is copied */
if (!boot_params.hdr.version)
- copy_bootdata(__va(real_mode_data));
+ copy_bootdata(real_mode_data);
x86_early_init_platform_quirks();
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index ac9d327d2e42..506de36293bc 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -266,9 +266,12 @@ ENTRY(secondary_startup_64)
movl initial_gs+4(%rip),%edx
wrmsr
- /* rsi is pointer to real mode structure with interesting info.
- pass it to C */
- movq %rsi, %rdi
+ /*
+ * %rsi is pointer to real mode struct boot_params with interesting info.
+ * Pass its virtual address to C.
+ */
+ movq $__PAGE_OFFSET_BASE, %rdi
+ addq %rsi, %rdi
.Ljump_to_C_code:
/*
@@ -487,6 +490,7 @@ early_gdt_descr:
early_gdt_descr_base:
.quad INIT_PER_CPU_VAR(gdt_page)
+/* The physical address the kernel is loaded at. */
ENTRY(phys_base)
/* This must match the first entry in level2_kernel_pgt */
.quad 0x0000000000000000
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] x86/head_64.S: Rename start_cpu()
2017-03-04 9:56 [PATCH 1/2] x86/head_64.S: Rename start_cpu() Borislav Petkov
2017-03-04 9:56 ` [PATCH 2/2] x86/head_64.S: Pass struct boot_params' virtual address to C Borislav Petkov
@ 2017-03-07 9:06 ` Ingo Molnar
2017-03-07 10:38 ` Borislav Petkov
2017-03-07 13:01 ` [tip:x86/boot] x86/boot/64: " tip-bot for Borislav Petkov
2 siblings, 1 reply; 8+ messages in thread
From: Ingo Molnar @ 2017-03-07 9:06 UTC (permalink / raw)
To: Borislav Petkov; +Cc: X86 ML, LKML, Thomas Gleixner
* Borislav Petkov <bp@alien8.de> wrote:
> From: Borislav Petkov <bp@suse.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@suse.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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] x86/head_64.S: Pass struct boot_params' virtual address to C
2017-03-04 9:56 ` [PATCH 2/2] x86/head_64.S: Pass struct boot_params' virtual address to C Borislav Petkov
@ 2017-03-07 9:11 ` Ingo Molnar
2017-03-07 10:40 ` Borislav Petkov
0 siblings, 1 reply; 8+ messages in thread
From: Ingo Molnar @ 2017-03-07 9:11 UTC (permalink / raw)
To: Borislav Petkov; +Cc: X86 ML, LKML, Thomas Gleixner
* Borislav Petkov <bp@alien8.de> wrote:
> From: Borislav Petkov <bp@suse.de>
>
> ... so that callees don't have to convert it and can use it directly.
> Simplifies C code a bit. Cleanup comments and formatting in the
> vicinity, while at it.
>
> Also, document what phys_base really is.
>
> No functionality change.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
> arch/x86/kernel/head64.c | 8 ++++----
> arch/x86/kernel/head_64.S | 10 +++++++---
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index 54a2372f5dbb..17ee20a9b4e4 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -118,7 +118,7 @@ static unsigned long get_cmd_line_ptr(void)
>
> static void __init copy_bootdata(char *real_mode_data)
> {
> - char * command_line;
> + char *command_line;
> unsigned long cmd_line_ptr;
>
> memcpy(&boot_params, real_mode_data, sizeof boot_params);
> @@ -130,7 +130,7 @@ static void __init copy_bootdata(char *real_mode_data)
> }
> }
>
> -asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
> +asmlinkage __visible void __init x86_64_start_kernel(char *real_mode_data)
> {
> int i;
>
> @@ -163,7 +163,7 @@ asmlinkage __visible void __init x86_64_start_kernel(char * real_mode_data)
> set_intr_gate(i, early_idt_handler_array[i]);
> load_idt((const struct desc_ptr *)&idt_descr);
>
> - copy_bootdata(__va(real_mode_data));
> + copy_bootdata(real_mode_data);
>
> /*
> * Load microcode early on BSP.
> @@ -180,7 +180,7 @@ void __init x86_64_start_reservations(char *real_mode_data)
> {
> /* version is always not zero if it is copied */
> if (!boot_params.hdr.version)
> - copy_bootdata(__va(real_mode_data));
> + copy_bootdata(real_mode_data);
>
> x86_early_init_platform_quirks();
>
> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> index ac9d327d2e42..506de36293bc 100644
> --- a/arch/x86/kernel/head_64.S
> +++ b/arch/x86/kernel/head_64.S
> @@ -266,9 +266,12 @@ ENTRY(secondary_startup_64)
> movl initial_gs+4(%rip),%edx
> wrmsr
>
> - /* rsi is pointer to real mode structure with interesting info.
> - pass it to C */
> - movq %rsi, %rdi
> + /*
> + * %rsi is pointer to real mode struct boot_params with interesting info.
> + * Pass its virtual address to C.
> + */
> + movq $__PAGE_OFFSET_BASE, %rdi
> + addq %rsi, %rdi
The updated comments and other details are fine, but I'm not sure about the __va()
change: the patch essentially open codes __va() in assembly - which would make any
changes to __va() [for whatever reason] more difficult, right?
Right now we don't seem to have a single such line of assembly, so changes to
__va() can be done in C alone.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] x86/head_64.S: Rename start_cpu()
2017-03-07 9:06 ` [PATCH 1/2] x86/head_64.S: Rename start_cpu() Ingo Molnar
@ 2017-03-07 10:38 ` Borislav Petkov
2017-03-07 12:56 ` Ingo Molnar
0 siblings, 1 reply; 8+ messages in thread
From: Borislav Petkov @ 2017-03-07 10:38 UTC (permalink / raw)
To: Ingo Molnar; +Cc: X86 ML, LKML, Thomas Gleixner
On Tue, Mar 07, 2017 at 10:06:38AM +0100, Ingo Molnar wrote:
> 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...
Sure, but other labels in this file don't have the preceding "_".
Perhaps we should agree on a convention for the local labels naming in
our asm and adhere to it...
Hmm.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] x86/head_64.S: Pass struct boot_params' virtual address to C
2017-03-07 9:11 ` Ingo Molnar
@ 2017-03-07 10:40 ` Borislav Petkov
0 siblings, 0 replies; 8+ messages in thread
From: Borislav Petkov @ 2017-03-07 10:40 UTC (permalink / raw)
To: Ingo Molnar; +Cc: X86 ML, LKML, Thomas Gleixner
On Tue, Mar 07, 2017 at 10:11:40AM +0100, Ingo Molnar wrote:
> The updated comments and other details are fine, but I'm not sure about the __va()
> change: the patch essentially open codes __va() in assembly - which would make any
> changes to __va() [for whatever reason] more difficult, right?
>
> Right now we don't seem to have a single such line of assembly, so changes to
> __va() can be done in C alone.
Ok, that's a fair point.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] x86/head_64.S: Rename start_cpu()
2017-03-07 10:38 ` Borislav Petkov
@ 2017-03-07 12:56 ` Ingo Molnar
0 siblings, 0 replies; 8+ messages in thread
From: Ingo Molnar @ 2017-03-07 12:56 UTC (permalink / raw)
To: Borislav Petkov; +Cc: X86 ML, LKML, Thomas Gleixner
* Borislav Petkov <bp@alien8.de> wrote:
> On Tue, Mar 07, 2017 at 10:06:38AM +0100, Ingo Molnar wrote:
> > 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...
>
> Sure, but other labels in this file don't have the preceding "_".
Yeah, and that would make it all inconsistent - fair enough.
Ok, I'll apply your patch as-is.
> Perhaps we should agree on a convention for the local labels naming in
> our asm and adhere to it...
Too much churn I suspect, plus we'd generate extra churn when people inevitably
get it wrong.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:x86/boot] x86/boot/64: Rename start_cpu()
2017-03-04 9:56 [PATCH 1/2] x86/head_64.S: Rename start_cpu() Borislav Petkov
2017-03-04 9:56 ` [PATCH 2/2] x86/head_64.S: Pass struct boot_params' virtual address to C Borislav Petkov
2017-03-07 9:06 ` [PATCH 1/2] x86/head_64.S: Rename start_cpu() Ingo Molnar
@ 2017-03-07 13:01 ` tip-bot for Borislav Petkov
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Borislav Petkov @ 2017-03-07 13:01 UTC (permalink / raw)
To: linux-tip-commits; +Cc: mingo, hpa, bp, peterz, tglx, linux-kernel, torvalds
Commit-ID: 79d243a042155b4421a06faaac15d775a133e6c8
Gitweb: http://git.kernel.org/tip/79d243a042155b4421a06faaac15d775a133e6c8
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Sat, 4 Mar 2017 10:56:10 +0100
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 7 Mar 2017 13:57:25 +0100
x86/boot/64: Rename start_cpu()
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@suse.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20170304095611.11355-1-bp@alien8.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
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 b467b14..ac9d327 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-03-07 14:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-04 9:56 [PATCH 1/2] x86/head_64.S: Rename start_cpu() Borislav Petkov
2017-03-04 9:56 ` [PATCH 2/2] x86/head_64.S: Pass struct boot_params' virtual address to C Borislav Petkov
2017-03-07 9:11 ` Ingo Molnar
2017-03-07 10:40 ` Borislav Petkov
2017-03-07 9:06 ` [PATCH 1/2] x86/head_64.S: Rename start_cpu() Ingo Molnar
2017-03-07 10:38 ` Borislav Petkov
2017-03-07 12:56 ` Ingo Molnar
2017-03-07 13:01 ` [tip:x86/boot] x86/boot/64: " tip-bot for Borislav Petkov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.