more arm64 early boot stuff
cover more arm64 early boot stuff
This is a bit like washing your car: you make one clean spot, and you have to go and wash the whole thing ...
Anyway, this is another couple of proposed improvements for head.S
Patch #1 removes the unnecessary split and branch of the MMU enable code.
Patch #2 is based on the kernel relocation preparatory patch that I sent yesterday, but reworked into something coherent, i.e., replace the open coded virt_to_phys() calculations with absolute/relative symbol references, as appropriate
Patch #3 adds code to warn when x1 .. x3 are not all zero as the boot protocol stipulates. This is to ensure the 'future use' these are reserved for can ever become a reality.
Ard Biesheuvel (3):
arm64: merge __enable_mmu and __turn_mmu_on
arm64: remove __calc_phys_offset
arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
arch/arm64/kernel/head.S | 87 +++++++++++++++-------------------------------- arch/arm64/kernel/setup.c | 13 +++++++ 2 files changed, 41 insertions(+), 59 deletions(-)
Signature · 1 line
1.8.3.2
PATCH 1/3 arm64: merge __enable_mmu and __turn_mmu_on
Enabling of the MMU is split into two functions, with an align and a branch in the middle. On arm64, the entire kernel Image is ID mapped so this is really not necessary, and we can just merge it into a single function.
--- arch/arm64/kernel/head.S | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-)
arch/arm64/kernel/head.S +8 -22
@@ -615,8 +615,13 @@ ENDPROC(__secondary_switched) #endif /* CONFIG_SMP */ /* - * Setup common bits before finally enabling the MMU. Essentially this is just - * loading the page table pointer and vector base registers. + * Enable the MMU. This completely changes the structure of the visible memory + * space. You will not be able to trace execution through this. + * + * x0 = system control register + * x27 = *virtual* address to jump to upon completion + * + * other registers depend on the function called upon completion * * On entry to this code, x0 must contain the SCTLR_EL1 value for turning on * the MMU.
@@ -627,29 +632,10 @@ __enable_mmu: msr ttbr0_el1, x25 // load TTBR0 msr ttbr1_el1, x26 // load TTBR1 isb - b __turn_mmu_on -ENDPROC(__enable_mmu) - -/* - * Enable the MMU. This completely changes the structure of the visible memory - * space. You will not be able to trace execution through this. - * - * x0 = system control register - * x27 = *virtual* address to jump to upon completion - * - * other registers depend on the function called upon completion - * - * We align the entire function to the smallest power of two larger than it to - * ensure it fits within a single block map entry. Otherwise were PHYS_OFFSET - * close to the end of a 512MB or 1GB block we might require an additional - * table to map the entire function. - */ - .align 4 -__turn_mmu_on: msr sctlr_el1, x0 isb br x27 -ENDPROC(__turn_mmu_on) +ENDPROC(__enable_mmu) /* * Calculate the start of physical memory.
Signature · 1 line
1.8.3.2
Mark Rutland
reviewerPATCH 1/3 arm64: merge __enable_mmu and __turn_mmu_on
Quoted 20 lines · Ard Biesheuvel
Enabling of the MMU is split into two functions, with an align and a branch in the middle. On arm64, the entire kernel Image is ID mapped so this is really not necessary, and we can just merge it into a single function.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm64/kernel/head.S | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-)arch/arm64/kernel/head.S +0
-2@@ -615,8 +615,13 @@ ENDPROC(__secondary_switched) #endif /* CONFIG_SMP */ /* - * Setup common bits before finally enabling the MMU. Essentially this is just - * loading the page table pointer and vector base registers.
arch/arm64/kernel/head.S ENDPROC(__secondary_switched) · line 627
Quoted 10 lines
arch/arm64/kernel/head.S +7
-0+ * Enable the MMU. This completely changes the structure of the visible memory + * space. You will not be able to trace execution through this. + * + * x0 = system control register + * x27 = *virtual* address to jump to upon completion + * + * other registers depend on the function called upon completion * * On entry to this code, x0 must contain the SCTLR_EL1 value for turning on * the MMU.
Minor nit: it's a bit redundant to have both comments regarding the sctlr_el1.
I'd recommed we drop/fold the 2nd comment into the first like:
x0 = SCTLR_EL1 value for turning on the MMU.
Otherwise, this looks good.
Mark.
Quoted 35 lines
arch/arm64/kernel/head.S +1
-20@@ -627,29 +632,10 @@ __enable_mmu: msr ttbr0_el1, x25 // load TTBR0 msr ttbr1_el1, x26 // load TTBR1 isb - b __turn_mmu_on -ENDPROC(__enable_mmu) - -/* - * Enable the MMU. This completely changes the structure of the visible memory - * space. You will not be able to trace execution through this. - * - * x0 = system control register - * x27 = *virtual* address to jump to upon completion - * - * other registers depend on the function called upon completion - * - * We align the entire function to the smallest power of two larger than it to - * ensure it fits within a single block map entry. Otherwise were PHYS_OFFSET - * close to the end of a 512MB or 1GB block we might require an additional - * table to map the entire function. - */ - .align 4 -__turn_mmu_on: msr sctlr_el1, x0 isb br x27 -ENDPROC(__turn_mmu_on) +ENDPROC(__enable_mmu) /* * Calculate the start of physical memory.Signature · 1 line
1.8.3.2
PATCH 1/3 arm64: merge __enable_mmu and __turn_mmu_on
In reply to Ard BiesheuvelQuoted 15 lines · Ard Biesheuvel
Enabling of the MMU is split into two functions, with an align and a branch in the middle. On arm64, the entire kernel Image is ID mapped so this is really not necessary, and we can just merge it into a single function.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm64/kernel/head.S | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-)arch/arm64/kernel/head.S +0
-0
arch/arm64/kernel/head.S ENDPROC(__secondary_switched) · line 619
Quoted 7 lines
arch/arm64/kernel/head.S +2
-2@@ -615,8 +615,13 @@ ENDPROC(__secondary_switched) #endif /* CONFIG_SMP */ /* - * Setup common bits before finally enabling the MMU. Essentially this is just - * loading the page table pointer and vector base registers. + * Enable the MMU. This completely changes the structure of the visible memory + * space. You will not be able to trace execution through this.
I don't understand the last sentence. I recall being able to read and eventually understand simulator instruction traces of this code. Is the sentence referring to the Embedded Trace Macrocell or something?
Chris
Signature · 3 lines
Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 1/3 arm64: merge __enable_mmu and __turn_mmu_on
Quoted 18 lines · Christopher Covington
Quoted 17 lines · Ard Biesheuvel
Enabling of the MMU is split into two functions, with an align and a branch in the middle. On arm64, the entire kernel Image is ID mapped so this is really not necessary, and we can just merge it into a single function.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm64/kernel/head.S | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-)arch/arm64/kernel/head.S +0
-0@@ -615,8 +615,13 @@ ENDPROC(__secondary_switched) #endif /* CONFIG_SMP */
Quoted 10 lines
arch/arm64/kernel/head.S ENDPROC(__secondary_switched) · line 619
Quoted 5 lines
arch/arm64/kernel/head.S +2
-2/* - * Setup common bits before finally enabling the MMU. Essentially this is just - * loading the page table pointer and vector base registers. + * Enable the MMU. This completely changes the structure of the visible memory + * space. You will not be able to trace execution through this.I don't understand the last sentence. I recall being able to read and eventually understand simulator instruction traces of this code. Is the sentence referring to the Embedded Trace Macrocell or something?
I guess the comment is a bit stale: it was inherited from the ARM version, where older platforms only have a single TTBR register, and switching address spaces is a bit more involved. On arm64, however, there are always two TTBR registers at EL1, and the address spaced they represent can never overlap, so there it isn't such a big deal.
Mark Rutland
reviewerPATCH 1/3 arm64: merge __enable_mmu and __turn_mmu_on
Quoted 10 lines
Quoted 9 lines
Quoted 4 lines
- * Setup common bits before finally enabling the MMU. Essentially this is just
- * loading the page table pointer and vector base registers.
+ * Enable the MMU. This completely changes the structure of the visible memory
+ * space. You will not be able to trace execution through this.I don't understand the last sentence. I recall being able to read and eventually understand simulator instruction traces of this code. Is the sentence referring to the Embedded Trace Macrocell or something?
Quoted 5 lines
I guess the comment is a bit stale: it was inherited from the ARM version, where older platforms only have a single TTBR register, and switching address spaces is a bit more involved. On arm64, however, there are always two TTBR registers at EL1, and the address spaced they represent can never overlap, so there it isn't such a big deal.
Indeed. Feel free to drop it if you want.
Mark.
PATCH 2/3 arm64: remove __calc_phys_offset
In reply to Ard BiesheuvelThis removes the function __calc_phys_offset and all open coded virtual to physical address translations using the offset kept in x28.
Instead, just use absolute or PC-relative symbol references as appropriate when referring to virtual or physical addresses, respectively.
--- arch/arm64/kernel/head.S | 49 ++++++++++++------------------------------------ 1 file changed, 12 insertions(+), 37 deletions(-)
arch/arm64/kernel/head.S +12 -37
@@ -36,8 +36,6 @@ #include <asm/page.h> #include <asm/virt.h> -#define KERNEL_RAM_VADDR (PAGE_OFFSET + TEXT_OFFSET) - #if (TEXT_OFFSET & 0xfff) != 0 #error TEXT_OFFSET must be at least 4KB aligned #elif (PAGE_OFFSET & 0x1fffff) != 0
@@ -46,13 +44,6 @@ #error TEXT_OFFSET must be less than 2MB #endif - .macro pgtbl, ttb0, ttb1, virt_to_phys - ldr \ttb1, =swapper_pg_dir - ldr \ttb0, =idmap_pg_dir - add \ttb1, \ttb1, \virt_to_phys - add \ttb0, \ttb0, \virt_to_phys - .endm - #ifdef CONFIG_ARM64_64K_PAGES #define BLOCK_SHIFT PAGE_SHIFT #define BLOCK_SIZE PAGE_SIZE
@@ -63,7 +54,7 @@ #define TABLE_SHIFT PUD_SHIFT #endif -#define KERNEL_START KERNEL_RAM_VADDR +#define KERNEL_START _text #define KERNEL_END _end /*
@@ -242,7 +233,7 @@ section_table: ENTRY(stext) mov x21, x0 // x21=FDT bl el2_setup // Drop to EL1, w20=cpu_boot_mode - bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET bl set_cpu_boot_mode_flag bl __vet_fdt
@@ -343,7 +334,8 @@ ENDPROC(__vet_fdt) * - pgd entry for fixed mappings (TTBR1) */ __create_page_tables: - pgtbl x25, x26, x28 // idmap_pg_dir and swapper_pg_dir addresses + adrp x25, idmap_pg_dir + adrp x26, swapper_pg_dir mov x27, lr /*
@@ -372,12 +364,10 @@ __create_page_tables: * Create the identity mapping. */ mov x0, x25 // idmap_pg_dir - ldr x3, =KERNEL_START - add x3, x3, x28 // __pa(KERNEL_START) + adrp x3, KERNEL_START // __pa(KERNEL_START) create_pgd_entry x0, x3, x5, x6 - ldr x6, =KERNEL_END mov x5, x3 // __pa(KERNEL_START) - add x6, x6, x28 // __pa(KERNEL_END) + adr_l x6, KERNEL_END // __pa(KERNEL_END) create_block_map x0, x7, x3, x5, x6 /*
@@ -386,7 +376,7 @@ __create_page_tables: mov x0, x26 // swapper_pg_dir mov x5, #PAGE_OFFSET create_pgd_entry x0, x5, x3, x6 - ldr x6, =KERNEL_END + ldr x6, =KERNEL_END // __va(KERNEL_END) mov x3, x24 // phys offset create_block_map x0, x7, x3, x5, x6
@@ -538,8 +528,7 @@ ENDPROC(el2_setup) * in x20. See arch/arm64/include/asm/virt.h for more info. */ ENTRY(set_cpu_boot_mode_flag) - ldr x1, =__boot_cpu_mode // Compute __boot_cpu_mode - add x1, x1, x28 + adr_l x1, __boot_cpu_mode cmp w20, #BOOT_CPU_MODE_EL2 b.ne 1f add x1, x1, #4
@@ -570,7 +559,7 @@ ENTRY(__boot_cpu_mode) */ ENTRY(secondary_holding_pen) bl el2_setup // Drop to EL1, w20=cpu_boot_mode - bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET bl set_cpu_boot_mode_flag mrs x0, mpidr_el1 ldr x1, =MPIDR_HWID_BITMASK
@@ -589,7 +578,7 @@ ENDPROC(secondary_holding_pen) */ ENTRY(secondary_entry) bl el2_setup // Drop to EL1 - bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET bl set_cpu_boot_mode_flag b secondary_startup ENDPROC(secondary_entry)
@@ -598,7 +587,8 @@ ENTRY(secondary_startup) /* * Common entry point for secondary CPUs. */ - pgtbl x25, x26, x28 // x25=TTBR0, x26=TTBR1 + adrp x25, idmap_pg_dir + adrp x26, swapper_pg_dir bl __cpu_setup // initialise processor ldr x21, =secondary_data
@@ -636,18 +626,3 @@ __enable_mmu: isb br x27 ENDPROC(__enable_mmu) - -/* - * Calculate the start of physical memory. - */ -__calc_phys_offset: - adr x0, 1f - ldp x1, x2, [x0] - sub x28, x0, x1 // x28 = PHYS_OFFSET - PAGE_OFFSET - add x24, x2, x28 // x24 = PHYS_OFFSET - ret -ENDPROC(__calc_phys_offset) - - .align 3 -1: .quad . - .quad PAGE_OFFSET
Signature · 1 line
1.8.3.2
Mark Rutland
reviewerPATCH 2/3 arm64: remove __calc_phys_offset
Quoted 50 lines · Ard Biesheuvel
This removes the function
__calc_phys_offsetand all open coded virtual to physical address translations using the offset kept in x28.Instead, just use absolute or PC-relative symbol references as appropriate when referring to virtual or physical addresses, respectively.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm64/kernel/head.S | 49 ++++++++++++------------------------------------ 1 file changed, 12 insertions(+), 37 deletions(-)arch/arm64/kernel/head.S +1
-10@@ -36,8 +36,6 @@ #include <asm/page.h> #include <asm/virt.h> -#define KERNEL_RAM_VADDR (PAGE_OFFSET + TEXT_OFFSET) - #if (TEXT_OFFSET & 0xfff) != 0 #error TEXT_OFFSET must be at least 4KB aligned #elif (PAGE_OFFSET & 0x1fffff) != 0@@ -46,13 +44,6 @@ #error TEXT_OFFSET must be less than 2MB #endif - .macro pgtbl, ttb0, ttb1, virt_to_phys - ldr \ttb1, =swapper_pg_dir - ldr \ttb0, =idmap_pg_dir - add \ttb1, \ttb1, \virt_to_phys - add \ttb0, \ttb0, \virt_to_phys - .endm - #ifdef CONFIG_ARM64_64K_PAGES #define BLOCK_SHIFT PAGE_SHIFT #define BLOCK_SIZE PAGE_SIZE@@ -63,7 +54,7 @@ #define TABLE_SHIFT PUD_SHIFT #endif -#define KERNEL_START KERNEL_RAM_VADDR +#define KERNEL_START _text #define KERNEL_END _end /*
arch/arm64/kernel/head.S section_table: · line 236
Quoted 5 lines
arch/arm64/kernel/head.S +1
-1@@ -242,7 +233,7 @@ section_table: ENTRY(stext) mov x21, x0 // x21=FDT bl el2_setup // Drop to EL1, w20=cpu_boot_mode - bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET
Neat!
Perhaps we could have:
#define PHYS_OFFSET (KERNEL_START - TEXT_OFFSET)
Which would make the all the PHYS_OFFSET calculations self-documenting. It shouldn't clash with the asm/memory.h definition because that's in an #ifndef __ASSEMBLY__ block.
Either way it all looks correct (and it's nice to see head.S shrink), so:
I assume you'll add this to the series introducing adr_l and friends?
Mark.
Quoted 98 lines
arch/arm64/kernel/head.S +10
-26bl set_cpu_boot_mode_flag bl __vet_fdt@@ -343,7 +334,8 @@ ENDPROC(__vet_fdt) * - pgd entry for fixed mappings (TTBR1) */ __create_page_tables: - pgtbl x25, x26, x28 // idmap_pg_dir and swapper_pg_dir addresses + adrp x25, idmap_pg_dir + adrp x26, swapper_pg_dir mov x27, lr /*@@ -372,12 +364,10 @@ __create_page_tables: * Create the identity mapping. */ mov x0, x25 // idmap_pg_dir - ldr x3, =KERNEL_START - add x3, x3, x28 // __pa(KERNEL_START) + adrp x3, KERNEL_START // __pa(KERNEL_START) create_pgd_entry x0, x3, x5, x6 - ldr x6, =KERNEL_END mov x5, x3 // __pa(KERNEL_START) - add x6, x6, x28 // __pa(KERNEL_END) + adr_l x6, KERNEL_END // __pa(KERNEL_END) create_block_map x0, x7, x3, x5, x6 /*@@ -386,7 +376,7 @@ __create_page_tables: mov x0, x26 // swapper_pg_dir mov x5, #PAGE_OFFSET create_pgd_entry x0, x5, x3, x6 - ldr x6, =KERNEL_END + ldr x6, =KERNEL_END // __va(KERNEL_END) mov x3, x24 // phys offset create_block_map x0, x7, x3, x5, x6@@ -538,8 +528,7 @@ ENDPROC(el2_setup) * in x20. See arch/arm64/include/asm/virt.h for more info. */ ENTRY(set_cpu_boot_mode_flag) - ldr x1, =__boot_cpu_mode // Compute __boot_cpu_mode - add x1, x1, x28 + adr_l x1, __boot_cpu_mode cmp w20, #BOOT_CPU_MODE_EL2 b.ne 1f add x1, x1, #4@@ -570,7 +559,7 @@ ENTRY(__boot_cpu_mode) */ ENTRY(secondary_holding_pen) bl el2_setup // Drop to EL1, w20=cpu_boot_mode - bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET bl set_cpu_boot_mode_flag mrs x0, mpidr_el1 ldr x1, =MPIDR_HWID_BITMASK@@ -589,7 +578,7 @@ ENDPROC(secondary_holding_pen) */ ENTRY(secondary_entry) bl el2_setup // Drop to EL1 - bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET bl set_cpu_boot_mode_flag b secondary_startup ENDPROC(secondary_entry)@@ -598,7 +587,8 @@ ENTRY(secondary_startup) /* * Common entry point for secondary CPUs. */ - pgtbl x25, x26, x28 // x25=TTBR0, x26=TTBR1 + adrp x25, idmap_pg_dir + adrp x26, swapper_pg_dir bl __cpu_setup // initialise processor ldr x21, =secondary_data@@ -636,18 +626,3 @@ __enable_mmu: isb br x27 ENDPROC(__enable_mmu) - -/* - * Calculate the start of physical memory. - */ -__calc_phys_offset: - adr x0, 1f - ldp x1, x2, [x0] - sub x28, x0, x1 // x28 = PHYS_OFFSET - PAGE_OFFSET - add x24, x2, x28 // x24 = PHYS_OFFSET - ret -ENDPROC(__calc_phys_offset) - - .align 3 -1: .quad . - .quad PAGE_OFFSETSignature · 1 line
1.8.3.2
PATCH 2/3 arm64: remove __calc_phys_offset
Quoted 59 lines · Mark Rutland
Quoted 50 lines · Ard Biesheuvel
This removes the function
__calc_phys_offsetand all open coded virtual to physical address translations using the offset kept in x28.Instead, just use absolute or PC-relative symbol references as appropriate when referring to virtual or physical addresses, respectively.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm64/kernel/head.S | 49 ++++++++++++------------------------------------ 1 file changed, 12 insertions(+), 37 deletions(-)arch/arm64/kernel/head.S +1
-10@@ -36,8 +36,6 @@ #include <asm/page.h> #include <asm/virt.h> -#define KERNEL_RAM_VADDR (PAGE_OFFSET + TEXT_OFFSET) - #if (TEXT_OFFSET & 0xfff) != 0 #error TEXT_OFFSET must be at least 4KB aligned #elif (PAGE_OFFSET & 0x1fffff) != 0@@ -46,13 +44,6 @@ #error TEXT_OFFSET must be less than 2MB #endif - .macro pgtbl, ttb0, ttb1, virt_to_phys - ldr \ttb1, =swapper_pg_dir - ldr \ttb0, =idmap_pg_dir - add \ttb1, \ttb1, \virt_to_phys - add \ttb0, \ttb0, \virt_to_phys - .endm - #ifdef CONFIG_ARM64_64K_PAGES #define BLOCK_SHIFT PAGE_SHIFT #define BLOCK_SIZE PAGE_SIZE@@ -63,7 +54,7 @@ #define TABLE_SHIFT PUD_SHIFT #endif -#define KERNEL_START KERNEL_RAM_VADDR +#define KERNEL_START _text #define KERNEL_END _end /*arch/arm64/kernel/head.S section_table: · line 236
Quoted 5 lines
arch/arm64/kernel/head.S +1
-1@@ -242,7 +233,7 @@ section_table: ENTRY(stext) mov x21, x0 // x21=FDT bl el2_setup // Drop to EL1, w20=cpu_boot_mode - bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSETNeat!
Quoted 8 lines
Perhaps we could have:
#define
PHYS_OFFSET(KERNEL_START-TEXT_OFFSET)Which would make the all the
PHYS_OFFSETcalculations self-documenting. It shouldn't clash with theasm/memory.hdefinition because that's in an #ifndef__ASSEMBLY__block.
I will use __PHYS_OFFSET instead, to prevent confusion.
Btw, I will drop the secondary 'adrp x24, __PHYS_OFFSET' lines completely, as the secondary boot path never refers to x24.
Quoted 5 lines
Either way it all looks correct (and it's nice to see
head.Sshrink), so:Reviewed-by Mark Rutland <[email protected]>
Thanks
Quoted 2 lines
I assume you'll add this to the series introducing
adr_land friends?
Yes, that is the idea. I will resend the entire series today, to round up the missing acks.
Signature · 1 line
Ard.
Quoted 98 lines
Quoted 98 lines
arch/arm64/kernel/head.S +10
-27bl set_cpu_boot_mode_flag bl __vet_fdt@@ -343,7 +334,8 @@ ENDPROC(__vet_fdt) * - pgd entry for fixed mappings (TTBR1) */ __create_page_tables: - pgtbl x25, x26, x28 // idmap_pg_dir and swapper_pg_dir addresses + adrp x25, idmap_pg_dir + adrp x26, swapper_pg_dir mov x27, lr /*@@ -372,12 +364,10 @@ __create_page_tables: * Create the identity mapping. */ mov x0, x25 // idmap_pg_dir - ldr x3, =KERNEL_START - add x3, x3, x28 // __pa(KERNEL_START) + adrp x3, KERNEL_START // __pa(KERNEL_START) create_pgd_entry x0, x3, x5, x6 - ldr x6, =KERNEL_END mov x5, x3 // __pa(KERNEL_START) - add x6, x6, x28 // __pa(KERNEL_END) + adr_l x6, KERNEL_END // __pa(KERNEL_END) create_block_map x0, x7, x3, x5, x6 /*@@ -386,7 +376,7 @@ __create_page_tables: mov x0, x26 // swapper_pg_dir mov x5, #PAGE_OFFSET create_pgd_entry x0, x5, x3, x6 - ldr x6, =KERNEL_END + ldr x6, =KERNEL_END // __va(KERNEL_END) mov x3, x24 // phys offset create_block_map x0, x7, x3, x5, x6@@ -538,8 +528,7 @@ ENDPROC(el2_setup) * in x20. See arch/arm64/include/asm/virt.h for more info. */ ENTRY(set_cpu_boot_mode_flag) - ldr x1, =__boot_cpu_mode // Compute __boot_cpu_mode - add x1, x1, x28 + adr_l x1, __boot_cpu_mode cmp w20, #BOOT_CPU_MODE_EL2 b.ne 1f add x1, x1, #4@@ -570,7 +559,7 @@ ENTRY(__boot_cpu_mode) */ ENTRY(secondary_holding_pen) bl el2_setup // Drop to EL1, w20=cpu_boot_mode - bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET bl set_cpu_boot_mode_flag mrs x0, mpidr_el1 ldr x1, =MPIDR_HWID_BITMASK@@ -589,7 +578,7 @@ ENDPROC(secondary_holding_pen) */ ENTRY(secondary_entry) bl el2_setup // Drop to EL1 - bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET bl set_cpu_boot_mode_flag b secondary_startup ENDPROC(secondary_entry)@@ -598,7 +587,8 @@ ENTRY(secondary_startup) /* * Common entry point for secondary CPUs. */ - pgtbl x25, x26, x28 // x25=TTBR0, x26=TTBR1 + adrp x25, idmap_pg_dir + adrp x26, swapper_pg_dir bl __cpu_setup // initialise processor ldr x21, =secondary_data@@ -636,18 +626,3 @@ __enable_mmu: isb br x27 ENDPROC(__enable_mmu) - -/* - * Calculate the start of physical memory. - */ -__calc_phys_offset: - adr x0, 1f - ldp x1, x2, [x0] - sub x28, x0, x1 // x28 = PHYS_OFFSET - PAGE_OFFSET - add x24, x2, x28 // x24 = PHYS_OFFSET - ret -ENDPROC(__calc_phys_offset) - - .align 3 -1: .quad . - .quad PAGE_OFFSET --1.8.3.2
Mark Rutland
reviewerPATCH 2/3 arm64: remove __calc_phys_offset
Quoted 14 lines
Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +1
-1- bl __calc_phys_offset // x24=PHYS_OFFSET, x28=PHYS_OFFSET-PAGE_OFFSET + adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSETNeat!
Quoted 8 lines
Perhaps we could have:
#define
PHYS_OFFSET(KERNEL_START-TEXT_OFFSET)Which would make the all the
PHYS_OFFSETcalculations self-documenting. It shouldn't clash with theasm/memory.hdefinition because that's in an #ifndef__ASSEMBLY__block.
Quoted 4 lines
I will use
__PHYS_OFFSETinstead, to prevent confusion.Btw, I will drop the secondary 'adrp x24,
__PHYS_OFFSET' lines completely, as the secondary boot path never refers to x24.
Sounds good to me on both counts.
Mark.
PATCH 3/3 arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
In reply to Ard BiesheuvelAccording to the arm64 boot protocol, registers x1 to x3 should be zero upon kernel entry, and non-zero values are reserved for future use. This future use is going to be problematic if we never enforce the current rules, so start enforcing them now, by emitting a warning if non-zero values are detected.
--- arch/arm64/kernel/head.S | 8 ++++++++ arch/arm64/kernel/setup.c | 13 +++++++++++++ 2 files changed, 21 insertions(+)
arch/arm64/kernel/head.S +8 -0
@@ -231,6 +231,10 @@ section_table: #endif ENTRY(stext) + adr x8, boot_regs // record the contents of + stp x0, x1, [x8] // x0 .. x3 at kernel entry + stp x2, x3, [x8, #16] + mov x21, x0 // x21=FDT bl el2_setup // Drop to EL1, w20=cpu_boot_mode adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET
@@ -251,6 +255,10 @@ ENTRY(stext) b __cpu_setup // initialise processor ENDPROC(stext) + .align 3 +ENTRY(boot_regs) + .skip 4 * 8 // x0 .. x3 + /* * Determine validity of the x21 FDT pointer. * The dtb must be 8-byte aligned and live in the first 512M of memory.
arch/arm64/kernel/setup.c +13 -0
@@ -532,3 +532,16 @@ const struct seq_operations cpuinfo_op = { .stop = c_stop, .show = c_show }; + +static int verify_boot_protocol (void ) +{ +extern u64 boot_regs[]; + +if (boot_regs[1 ] || boot_regs[2 ] || boot_regs[3 ]) { +pr_err ("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n" , + boot_regs[1 ], boot_regs[2 ], boot_regs[3 ]); +pr_err ("WARNING: your bootloader may fail to load newer kernels\n" ); + } +return 0 ; +} +late_initcall (verify_boot_protocol);
Signature · 1 line
1.8.3.2
Mark Rutland
reviewerPATCH 3/3 arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
Quoted 17 lines · Ard Biesheuvel
According to the arm64 boot protocol, registers x1 to x3 should be zero upon kernel entry, and non-zero values are reserved for future use. This future use is going to be problematic if we never enforce the current rules, so start enforcing them now, by emitting a warning if non-zero values are detected.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm64/kernel/head.S | 8 ++++++++ arch/arm64/kernel/setup.c | 13 +++++++++++++ 2 files changed, 21 insertions(+)arch/arm64/kernel/head.S +0
-0
arch/arm64/kernel/head.S section_table: · line 236
Quoted 6 lines
arch/arm64/kernel/head.S +3
-0@@ -231,6 +231,10 @@ section_table: #endif ENTRY(stext) + adr x8, boot_regs // record the contents of + stp x0, x1, [x8] // x0 .. x3 at kernel entry + stp x2, x3, [x8, #16]
These accesses will be non-cacheable, but the caches could have clean entries for this PA range.
Without an invalidate between the writes and reads, we might see stale values (the other option is to preserve the registers across enabling the MMU).
arch/arm64/kernel/head.S ENTRY(stext) · line 260
Quoted 11 lines
arch/arm64/kernel/head.S +4
-0+ mov x21, x0 // x21=FDT bl el2_setup // Drop to EL1, w20=cpu_boot_mode adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET@@ -251,6 +255,10 @@ ENTRY(stext) b __cpu_setup // initialise processor ENDPROC(stext) + .align 3 +ENTRY(boot_regs) + .skip 4 * 8 // x0 .. x3
Can't we can drop this here...
arch/arm64/kernel/setup.c const struct seq_operations cpuinfo_op = { · line 534
Quoted 12 lines
arch/arm64/kernel/head.S +1
-0+ /* * Determine validity of the x21 FDT pointer. * The dtb must be 8-byte aligned and live in the first 512M of memory.arch/arm64/kernel/setup.c +0
-0@@ -532,3 +532,16 @@ const struct seq_operations cpuinfo_op = { .stop = c_stop, .show = c_show };
... and here have
u64 boot_regs[4];
Using adrp + lo12 to address the symbol?
arch/arm64/kernel/setup.c const struct seq_operations cpuinfo_op = { · line 547
Quoted 13 lines
arch/arm64/kernel/setup.c +13
-0+ +static int verify_boot_protocol (void ) +{ +extern u64 boot_regs[]; + +if (boot_regs[1 ] || boot_regs[2 ] || boot_regs[3 ]) { +pr_err ("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n" , + boot_regs[1 ], boot_regs[2 ], boot_regs[3 ]); +pr_err ("WARNING: your bootloader may fail to load newer kernels\n" ); + } +return 0 ; +} +late_initcall (verify_boot_protocol);
I think it would be nicer to plumb this into setup_arch rather than an initcall. That way the warning will be at a consistent, prominent location in the log.
Thanks,
Mark.
PATCH 3/3 arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
In reply to Ard BiesheuvelQuoted 45 lines · Ard Biesheuvel
According to the arm64 boot protocol, registers x1 to x3 should be zero upon kernel entry, and non-zero values are reserved for future use. This future use is going to be problematic if we never enforce the current rules, so start enforcing them now, by emitting a warning if non-zero values are detected.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm64/kernel/head.S | 8 ++++++++ arch/arm64/kernel/setup.c | 13 +++++++++++++ 2 files changed, 21 insertions(+)arch/arm64/kernel/head.S +8
-0@@ -231,6 +231,10 @@ section_table: #endif ENTRY(stext) + adr x8, boot_regs // record the contents of + stp x0, x1, [x8] // x0 .. x3 at kernel entry + stp x2, x3, [x8, #16] + mov x21, x0 // x21=FDT bl el2_setup // Drop to EL1, w20=cpu_boot_mode adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET@@ -251,6 +255,10 @@ ENTRY(stext) b __cpu_setup // initialise processor ENDPROC(stext) + .align 3 +ENTRY(boot_regs) + .skip 4 * 8 // x0 .. x3 + /* * Determine validity of the x21 FDT pointer. * The dtb must be 8-byte aligned and live in the first 512M of memory.arch/arm64/kernel/setup.c +0
-0@@ -532,3 +532,16 @@ const struct seq_operations cpuinfo_op = { .stop = c_stop, .show = c_show
arch/arm64/kernel/setup.c const struct seq_operations cpuinfo_op = { · line 543
Quoted 10 lines
arch/arm64/kernel/setup.c +9
-0}; + +static int verify_boot_protocol (void ) +{ +extern u64 boot_regs[]; + +if (boot_regs[1 ] || boot_regs[2 ] || boot_regs[3 ]) { +pr_err ("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n" , + boot_regs[1 ], boot_regs[2 ], boot_regs[3 ]); +pr_err ("WARNING: your bootloader may fail to load newer kernels\n" );
pr_warn?
arch/arm64/kernel/setup.c const struct seq_operations cpuinfo_op = { · line 547
Quoted 5 lines
arch/arm64/kernel/setup.c +4
-0+ } +return 0 ; +} +late_initcall (verify_boot_protocol);
Chris
Signature · 3 lines
Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH 3/3 arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
Quoted 50 lines · Christopher Covington
Quoted 49 lines · Ard Biesheuvel
According to the arm64 boot protocol, registers x1 to x3 should be zero upon kernel entry, and non-zero values are reserved for future use. This future use is going to be problematic if we never enforce the current rules, so start enforcing them now, by emitting a warning if non-zero values are detected.
Signed-off-by Ard Biesheuvel <[email protected]>--- arch/arm64/kernel/head.S | 8 ++++++++ arch/arm64/kernel/setup.c | 13 +++++++++++++ 2 files changed, 21 insertions(+)arch/arm64/kernel/head.S +8
-0@@ -231,6 +231,10 @@ section_table: #endif ENTRY(stext) + adr x8, boot_regs // record the contents of + stp x0, x1, [x8] // x0 .. x3 at kernel entry + stp x2, x3, [x8, #16] + mov x21, x0 // x21=FDT bl el2_setup // Drop to EL1, w20=cpu_boot_mode adrp x24, KERNEL_START - TEXT_OFFSET // x24=PHYS_OFFSET@@ -251,6 +255,10 @@ ENTRY(stext) b __cpu_setup // initialise processor ENDPROC(stext) + .align 3 +ENTRY(boot_regs) + .skip 4 * 8 // x0 .. x3 + /* * Determine validity of the x21 FDT pointer. * The dtb must be 8-byte aligned and live in the first 512M of memory.arch/arm64/kernel/setup.c +3
-0@@ -532,3 +532,16 @@ const struct seq_operations cpuinfo_op = { .stop = c_stop, .show = c_show }; + +static int verify_boot_protocol (void ) +{
Quoted 9 lines
arch/arm64/kernel/setup.c const struct seq_operations cpuinfo_op = { · line 543
Quoted 6 lines
arch/arm64/kernel/setup.c +6
-0+extern u64 boot_regs[]; + +if (boot_regs[1 ] || boot_regs[2 ] || boot_regs[3 ]) { +pr_err ("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n" , + boot_regs[1 ], boot_regs[2 ], boot_regs[3 ]); +pr_err ("WARNING: your bootloader may fail to load newer kernels\n" );
pr_warn?
Semantically more correct, perhaps, but pr_err() should be slightly noisier, which is preferred here imo
Quoted 12 lines
arch/arm64/kernel/setup.c const struct seq_operations cpuinfo_op = { · line 547
Quoted 5 lines
arch/arm64/kernel/setup.c +4
-0+ } +return 0 ; +} +late_initcall (verify_boot_protocol);Chris
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project