Skip to contentSkip to conversation

arm64: head.S cleanup

PATCH Superseded Ard Biesheuvel 3 participants 8 messages Mar 4, 2015 arm64

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Ard Biesheuvel

author
#0 lore raw

v2 cover arm64: head.S cleanup

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(-)
Signature · 1 line
1.8.3.2
author
#1 lore raw

PATCH v2 1/3 arm64: remove processor_id

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 <[email protected]>
---
 arch/arm64/kernel/head.S  | 10 +++-------
 arch/arm64/kernel/setup.c |  3 ---
 2 files changed, 3 insertions(+), 10 deletions(-)
arch/arm64/kernel/head.S +3 -7
index 8ce88e08c030..003db2eadd7a 100644
@@ -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)?
arch/arm64/kernel/setup.c +0 -3
index e8420f635bd4..8b82ef19b81b 100644
@@ -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);
Signature · 1 line
1.8.3.2
reviewer
#2 lore raw

PATCH v2 1/3 arm64: remove processor_id

Hi Ard,

Quoted 2 lines · 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.

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.

Quoted 31 lines

So remove it.

Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/head.S  | 10 +++-------
 arch/arm64/kernel/setup.c |  3 ---
 2 files changed, 3 insertions(+), 10 deletions(-)
arch/arm64/kernel/head.S +1 -3
index 8ce88e08c030..003db2eadd7a 100644
@@ -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

arch/arm64/kernel/head.S __mmap_switched: · line 459

Quoted 6 lines
arch/arm64/kernel/head.S +1 -1
@@ -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 <[email protected]>

Mark.

Quoted 34 lines
arch/arm64/kernel/head.S +1 -3
 	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)?
arch/arm64/kernel/setup.c +0 -3
index e8420f635bd4..8b82ef19b81b 100644
@@ -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);
Signature · 1 line
1.8.3.2
author
#4 lore raw

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

In reply to 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 <[email protected]>
---
 arch/arm64/kernel/head.S | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)
arch/arm64/kernel/head.S +13 -21
index 003db2eadd7a..e6216b56257a 100644
@@ -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)
Signature · 1 line
1.8.3.2
author
#6 lore raw

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

In reply to Ard Biesheuvel

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

Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kernel/head.S | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)
arch/arm64/kernel/head.S +2 -14
index e6216b56257a..89444070f96b 100644
@@ -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
Signature · 1 line
1.8.3.2
maintainer
#7 lore raw

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

Quoted 4 lines · Ard Biesheuvel

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

Signed-off-by Ard Biesheuvel <[email protected]>
Reviewed-by Catalin Marinas <[email protected]>

arch/arm64/kernel/head.S ENDPROC(__error) · line 720

Quoted 7 lines

ENTRY(lookup_processor_type)

arch/arm64/kernel/head.S +2 -4
-	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.

Signature · 1 line
Catalin