* [PATCH v2 0/3] arm64: head.S cleanup
@ 2015-03-04 11:23 Ard Biesheuvel
2015-03-04 11:23 ` [PATCH v2 1/3] arm64: remove processor_id Ard Biesheuvel
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2015-03-04 11:23 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2 1/3] arm64: remove processor_id 2015-03-04 11:23 [PATCH v2 0/3] arm64: head.S cleanup Ard Biesheuvel @ 2015-03-04 11:23 ` Ard Biesheuvel 2015-03-04 11:57 ` Mark Rutland 2015-03-04 18:08 ` Catalin Marinas 2015-03-04 11:23 ` [PATCH v2 2/3] arm64: remove __switch_data object from head.S Ard Biesheuvel 2015-03-04 11:23 ` [PATCH v2 3/3] arm64: remove __lookup_processor_type_data " Ard Biesheuvel 2 siblings, 2 replies; 8+ messages in thread From: Ard Biesheuvel @ 2015-03-04 11:23 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 1/3] arm64: remove processor_id 2015-03-04 11:23 ` [PATCH v2 1/3] arm64: remove processor_id Ard Biesheuvel @ 2015-03-04 11:57 ` Mark Rutland 2015-03-04 18:08 ` Catalin Marinas 1 sibling, 0 replies; 8+ messages in thread From: Mark Rutland @ 2015-03-04 11:57 UTC (permalink / raw) To: linux-arm-kernel 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/3] arm64: remove processor_id 2015-03-04 11:23 ` [PATCH v2 1/3] arm64: remove processor_id Ard Biesheuvel 2015-03-04 11:57 ` Mark Rutland @ 2015-03-04 18:08 ` Catalin Marinas 1 sibling, 0 replies; 8+ messages in thread From: Catalin Marinas @ 2015-03-04 18:08 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] arm64: remove __switch_data object from head.S 2015-03-04 11:23 [PATCH v2 0/3] arm64: head.S cleanup Ard Biesheuvel 2015-03-04 11:23 ` [PATCH v2 1/3] arm64: remove processor_id Ard Biesheuvel @ 2015-03-04 11:23 ` Ard Biesheuvel 2015-03-04 18:08 ` Catalin Marinas 2015-03-04 11:23 ` [PATCH v2 3/3] arm64: remove __lookup_processor_type_data " Ard Biesheuvel 2 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2015-03-04 11:23 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/3] arm64: remove __switch_data object from head.S 2015-03-04 11:23 ` [PATCH v2 2/3] arm64: remove __switch_data object from head.S Ard Biesheuvel @ 2015-03-04 18:08 ` Catalin Marinas 0 siblings, 0 replies; 8+ messages in thread From: Catalin Marinas @ 2015-03-04 18:08 UTC (permalink / raw) To: linux-arm-kernel 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> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] arm64: remove __lookup_processor_type_data object from head.S 2015-03-04 11:23 [PATCH v2 0/3] arm64: head.S cleanup Ard Biesheuvel 2015-03-04 11:23 ` [PATCH v2 1/3] arm64: remove processor_id Ard Biesheuvel 2015-03-04 11:23 ` [PATCH v2 2/3] arm64: remove __switch_data object from head.S Ard Biesheuvel @ 2015-03-04 11:23 ` Ard Biesheuvel 2015-03-04 18:09 ` Catalin Marinas 2 siblings, 1 reply; 8+ messages in thread From: Ard Biesheuvel @ 2015-03-04 11:23 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/3] arm64: remove __lookup_processor_type_data object from head.S 2015-03-04 11:23 ` [PATCH v2 3/3] arm64: remove __lookup_processor_type_data " Ard Biesheuvel @ 2015-03-04 18:09 ` Catalin Marinas 0 siblings, 0 replies; 8+ messages in thread From: Catalin Marinas @ 2015-03-04 18:09 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-03-04 18:09 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-04 11:23 [PATCH v2 0/3] arm64: head.S cleanup Ard Biesheuvel 2015-03-04 11:23 ` [PATCH v2 1/3] arm64: remove processor_id Ard Biesheuvel 2015-03-04 11:57 ` Mark Rutland 2015-03-04 18:08 ` Catalin Marinas 2015-03-04 11:23 ` [PATCH v2 2/3] arm64: remove __switch_data object from head.S Ard Biesheuvel 2015-03-04 18:08 ` Catalin Marinas 2015-03-04 11:23 ` [PATCH v2 3/3] arm64: remove __lookup_processor_type_data " Ard Biesheuvel 2015-03-04 18:09 ` Catalin Marinas
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.