[PATCH v2 0/3] arm64: head.S cleanup

[PATCH v2 0/3] arm64: head.S cleanup

[Ard Biesheuvel]

This some janitorial work on head.S, just stuff I noticed when making
changes to it for other reasons.

Some may find the churn pointless, but my opinion is that .S files -if they
cannot be avoided in the first place- should be as simple and straightforward
as possible.

Ard Biesheuvel (3):
  arm64: remove processor_id
  arm64: remove __switch_data object from head.S
  arm64: remove __lookup_processor_type_data, object from head.S

 arch/arm64/kernel/head.S  | 58 ++++++++++++++---------------------------------
 arch/arm64/kernel/setup.c |  3 ---
 2 files changed, 17 insertions(+), 44 deletions(-)

-- 
1.8.3.2

[PATCH v2 1/3] arm64: remove processor_id

[Ard Biesheuvel]

The global processor_id is assigned the MIDR_EL1 value of the boot
CPU in the early init code, but is never referenced afterwards.

So remove it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/head.S  | 10 +++-------
 arch/arm64/kernel/setup.c |  3 ---
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 8ce88e08c030..003db2eadd7a 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -244,8 +244,7 @@ ENTRY(stext)
 	bl	el2_setup			// Drop to EL1, w20=cpu_boot_mode
 	bl	__calc_phys_offset		// x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET
 	bl	set_cpu_boot_mode_flag
-	mrs	x22, midr_el1			// x22=cpuid
-	mov	x0, x22
+	mrs	x0, midr_el1
 	bl	lookup_processor_type
 	mov	x23, x0				// x23=current cpu_table
 	/*
@@ -439,7 +438,6 @@ __switch_data:
 	.quad	__mmap_switched
 	.quad	__bss_start			// x6
 	.quad	__bss_stop			// x7
-	.quad	processor_id			// x4
 	.quad	__fdt_pointer			// x5
 	.quad	memstart_addr			// x6
 	.quad	init_thread_union + THREAD_START_SP // sp
@@ -457,11 +455,10 @@ __mmap_switched:
 	str	xzr, [x6], #8			// Clear BSS
 	b	1b
 2:
-	ldp	x4, x5, [x3], #16
+	ldr	x5, [x3], #8
 	ldr	x6, [x3], #8
 	ldr	x16, [x3]
 	mov	sp, x16
-	str	x22, [x4]			// Save processor ID
 	str	x21, [x5]			// Save FDT pointer
 	str	x24, [x6]			// Save PHYS_OFFSET
 	mov	x29, #0
@@ -633,8 +630,7 @@ ENTRY(secondary_startup)
 	/*
 	 * Common entry point for secondary CPUs.
 	 */
-	mrs	x22, midr_el1			// x22=cpuid
-	mov	x0, x22
+	mrs	x0, midr_el1
 	bl	lookup_processor_type
 	mov	x23, x0				// x23=current cpu_table
 	cbz	x23, __error_p			// invalid processor (x23=0)?
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index e8420f635bd4..8b82ef19b81b 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -63,9 +63,6 @@
 #include <asm/psci.h>
 #include <asm/efi.h>
 
-unsigned int processor_id;
-EXPORT_SYMBOL(processor_id);
-
 unsigned long elf_hwcap __read_mostly;
 EXPORT_SYMBOL_GPL(elf_hwcap);
 
-- 
1.8.3.2

[PATCH v2 2/3] arm64: remove __switch_data object from head.S

[Ard Biesheuvel]

This removes the confusing __switch_data object from head.S,
and replaces it with standard PC-relative references to the
various symbols it encapsulates.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/head.S | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 003db2eadd7a..e6216b56257a 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -263,7 +263,7 @@ ENTRY(stext)
 	 * On return, the CPU will be ready for the MMU to be turned on and
 	 * the TCR will have been set.
 	 */
-	ldr	x27, __switch_data		// address to jump to after
+	ldr	x27, =__mmap_switched		// address to jump to after
 						// MMU has been enabled
 	adrp	lr, __enable_mmu		// return (PIC) address
 	add	lr, lr, #:lo12:__enable_mmu
@@ -432,35 +432,27 @@ __create_page_tables:
 ENDPROC(__create_page_tables)
 	.ltorg
 
-	.align	3
-	.type	__switch_data, %object
-__switch_data:
-	.quad	__mmap_switched
-	.quad	__bss_start			// x6
-	.quad	__bss_stop			// x7
-	.quad	__fdt_pointer			// x5
-	.quad	memstart_addr			// x6
-	.quad	init_thread_union + THREAD_START_SP // sp
-
 /*
- * The following fragment of code is executed with the MMU on in MMU mode, and
- * uses absolute addresses; this is not position independent.
+ * The following fragment of code is executed with the MMU enabled.
  */
+	.set	initial_sp, init_thread_union + THREAD_START_SP
 __mmap_switched:
-	adr	x3, __switch_data + 8
+	adrp	x6, __bss_start
+	adrp	x7, __bss_stop
+	add	x6, x6, :lo12:__bss_start
+	add	x7, x7, :lo12:__bss_stop
 
-	ldp	x6, x7, [x3], #16
 1:	cmp	x6, x7
 	b.hs	2f
 	str	xzr, [x6], #8			// Clear BSS
 	b	1b
 2:
-	ldr	x5, [x3], #8
-	ldr	x6, [x3], #8
-	ldr	x16, [x3]
-	mov	sp, x16
-	str	x21, [x5]			// Save FDT pointer
-	str	x24, [x6]			// Save PHYS_OFFSET
+	adrp	x4, initial_sp
+	adrp	x5, __fdt_pointer
+	adrp	x6, memstart_addr
+	add	sp, x4, :lo12:initial_sp
+	str	x21, [x5, :lo12:__fdt_pointer]	// Save FDT pointer
+	str	x24, [x6, :lo12:memstart_addr]	// Save PHYS_OFFSET
 	mov	x29, #0
 	b	start_kernel
 ENDPROC(__mmap_switched)
-- 
1.8.3.2

[PATCH v2 3/3] arm64: remove __lookup_processor_type_data object from head.S

[Ard Biesheuvel]

This removes __lookup_processor_type_data, replacing the original
use with a simple PC-relative address load.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/kernel/head.S | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index e6216b56257a..89444070f96b 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -714,15 +714,10 @@ ENDPROC(__error)
  * This function gets the processor ID in w0 and searches the cpu_table[] for
  * a match. It returns a pointer to the struct cpu_info it found. The
  * cpu_table[] must end with an empty (all zeros) structure.
- *
- * This routine can be called via C code and it needs to work with the MMU
- * both disabled and enabled (the offset is calculated automatically).
  */
 ENTRY(lookup_processor_type)
-	adr	x1, __lookup_processor_type_data
-	ldp	x2, x3, [x1]
-	sub	x1, x1, x2			// get offset between VA and PA
-	add	x3, x3, x1			// convert VA to PA
+	adrp	x3, cpu_table
+	add	x3, x3, :lo12:cpu_table
 1:
 	ldp	w5, w6, [x3]			// load cpu_id_val and cpu_id_mask
 	cbz	w5, 2f				// end of list?
@@ -737,10 +732,3 @@ ENTRY(lookup_processor_type)
 	mov	x0, x3
 	ret
 ENDPROC(lookup_processor_type)
-
-	.align	3
-	.type	__lookup_processor_type_data, %object
-__lookup_processor_type_data:
-	.quad	.
-	.quad	cpu_table
-	.size	__lookup_processor_type_data, . - __lookup_processor_type_data
-- 
1.8.3.2

[PATCH v2 1/3] arm64: remove processor_id

[Mark Rutland]

Hi Ard,

On Wed, Mar 04, 2015 at 11:23:15AM +0000, Ard Biesheuvel wrote:
> The global processor_id is assigned the MIDR_EL1 value of the boot
> CPU in the early init code, but is never referenced afterwards.

I think it's also worth noting that it's also non-sensical and
misleading for big.LITTLE systems, and getting rid of it removes the
potential for confusion.

> So remove it.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/kernel/head.S  | 10 +++-------
>  arch/arm64/kernel/setup.c |  3 ---
>  2 files changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 8ce88e08c030..003db2eadd7a 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -244,8 +244,7 @@ ENTRY(stext)
>  	bl	el2_setup			// Drop to EL1, w20=cpu_boot_mode
>  	bl	__calc_phys_offset		// x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET
>  	bl	set_cpu_boot_mode_flag
> -	mrs	x22, midr_el1			// x22=cpuid
> -	mov	x0, x22
> +	mrs	x0, midr_el1
>  	bl	lookup_processor_type
>  	mov	x23, x0				// x23=current cpu_table
>  	/*
> @@ -439,7 +438,6 @@ __switch_data:
>  	.quad	__mmap_switched
>  	.quad	__bss_start			// x6
>  	.quad	__bss_stop			// x7
> -	.quad	processor_id			// x4
>  	.quad	__fdt_pointer			// x5
>  	.quad	memstart_addr			// x6
>  	.quad	init_thread_union + THREAD_START_SP // sp
> @@ -457,11 +455,10 @@ __mmap_switched:
>  	str	xzr, [x6], #8			// Clear BSS
>  	b	1b
>  2:
> -	ldp	x4, x5, [x3], #16
> +	ldr	x5, [x3], #8
>  	ldr	x6, [x3], #8

Here it would be possible to instead do:

-	ldp	x4, x5, [x3], #16
+	ldp	x5, x6, [x3], #16
-	ldr	x6, [x3], #8

But given this will change in a later patch anyway I guess it doesn't
matter either way.

Other that those comments, the patch looks good to me:

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Mark.

>  	ldr	x16, [x3]
>  	mov	sp, x16
> -	str	x22, [x4]			// Save processor ID
>  	str	x21, [x5]			// Save FDT pointer
>  	str	x24, [x6]			// Save PHYS_OFFSET
>  	mov	x29, #0
> @@ -633,8 +630,7 @@ ENTRY(secondary_startup)
>  	/*
>  	 * Common entry point for secondary CPUs.
>  	 */
> -	mrs	x22, midr_el1			// x22=cpuid
> -	mov	x0, x22
> +	mrs	x0, midr_el1
>  	bl	lookup_processor_type
>  	mov	x23, x0				// x23=current cpu_table
>  	cbz	x23, __error_p			// invalid processor (x23=0)?
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index e8420f635bd4..8b82ef19b81b 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -63,9 +63,6 @@
>  #include <asm/psci.h>
>  #include <asm/efi.h>
>  
> -unsigned int processor_id;
> -EXPORT_SYMBOL(processor_id);
> -
>  unsigned long elf_hwcap __read_mostly;
>  EXPORT_SYMBOL_GPL(elf_hwcap);
>  
> -- 
> 1.8.3.2
> 
> 

[PATCH v2 1/3] arm64: remove processor_id

[Catalin Marinas]

On Wed, Mar 04, 2015 at 12:23:15PM +0100, Ard Biesheuvel wrote:
> The global processor_id is assigned the MIDR_EL1 value of the boot
> CPU in the early init code, but is never referenced afterwards.
> 
> So remove it.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH v2 2/3] arm64: remove __switch_data object from head.S

[Catalin Marinas]

On Wed, Mar 04, 2015 at 12:23:16PM +0100, Ard Biesheuvel wrote:
> This removes the confusing __switch_data object from head.S,
> and replaces it with standard PC-relative references to the
> various symbols it encapsulates.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH v2 3/3] arm64: remove __lookup_processor_type_data object from head.S

[Catalin Marinas]

On Wed, Mar 04, 2015 at 12:23:17PM +0100, Ard Biesheuvel wrote:
> This removes __lookup_processor_type_data, replacing the original
> use with a simple PC-relative address load.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

>  ENTRY(lookup_processor_type)
> -	adr	x1, __lookup_processor_type_data
> -	ldp	x2, x3, [x1]
> -	sub	x1, x1, x2			// get offset between VA and PA
> -	add	x3, x3, x1			// convert VA to PA
> +	adrp	x3, cpu_table
> +	add	x3, x3, :lo12:cpu_table

I wonder whether we should add a macro for these two instructions, it's
becoming a common pattern.

-- 
Catalin