* [PATCH v5 0/8] arm64: head.S cleanup
@ 2015-03-18 14:55 Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 1/8] arm64: Get rid of struct cpu_table Ard Biesheuvel
` (8 more replies)
0 siblings, 9 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 14:55 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.
This still needs acks on patches #2, #5 and #8
Changes since v4:
- included Marc Zyngier's struct cpu_table removal patch
- dropped __lookup_processor_type_data patch now that Marc has nuked the
whole thing
- added patch to merge __enable_mmu() and __turn_mmu_on() into a single
function, as there is no need for funky trampoline stuff when you have
two non-overlapping TTBRs
- added patch to remove __calc_phys_offset and use PC-relative references or
absolute (linker generated) references as appropriate
- added patch to complain when x1/x2/x3 are non-zero, which helps ensure that
we will ever be able to use them for anything
- added R-b's
Changes since v3:
- added similar patch for secondary_holding_pen_release
- fixed bug in patch #1 (ldr_l)
Changes since v2:
- added separate patch to add macros for adrp/add, adrp/ldr and adrp/str
- added R-b's
Ard Biesheuvel (7):
arm64: add macros for common adrp usages
arm64: remove processor_id
arm64: remove __switch_data object from head.S
arm64: use PC-relative reference for secondary_holding_pen_release
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
Marc Zyngier (1):
arm64: Get rid of struct cpu_table
arch/arm64/include/asm/assembler.h | 29 ++++++
arch/arm64/include/asm/cputable.h | 30 ------
arch/arm64/kernel/Makefile | 2 +-
arch/arm64/kernel/asm-offsets.c | 4 -
arch/arm64/kernel/cputable.c | 33 ------
arch/arm64/kernel/head.S | 203 +++++++------------------------------
arch/arm64/kernel/setup.c | 34 +++----
7 files changed, 83 insertions(+), 252 deletions(-)
delete mode 100644 arch/arm64/include/asm/cputable.h
delete mode 100644 arch/arm64/kernel/cputable.c
--
1.8.3.2
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 1/8] arm64: Get rid of struct cpu_table
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
@ 2015-03-18 14:55 ` Ard Biesheuvel
2015-03-18 16:11 ` Mark Rutland
2015-03-23 17:11 ` Suzuki K. Poulose
2015-03-18 14:55 ` [PATCH v5 2/8] arm64: add macros for common adrp usages Ard Biesheuvel
` (7 subsequent siblings)
8 siblings, 2 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 14:55 UTC (permalink / raw)
To: linux-arm-kernel
From: Marc Zyngier <marc.zyngier@arm.com>
struct cpu_table is an artifact left from the (very) early days of
the arm64 port, and its only real use is to allow the most beautiful
"AArch64 Processor" string to be displayed at boot time.
Really? Yes, really.
Let's get rid of it. In order to avoid another BogoMips-gate, the
aforementioned string is preserved.
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/cputable.h | 30 ----------------
arch/arm64/kernel/Makefile | 2 +-
arch/arm64/kernel/asm-offsets.c | 4 ---
arch/arm64/kernel/cputable.c | 33 -----------------
arch/arm64/kernel/head.S | 76 +++------------------------------------
arch/arm64/kernel/setup.c | 16 ++-------
6 files changed, 8 insertions(+), 153 deletions(-)
delete mode 100644 arch/arm64/include/asm/cputable.h
delete mode 100644 arch/arm64/kernel/cputable.c
diff --git a/arch/arm64/include/asm/cputable.h b/arch/arm64/include/asm/cputable.h
deleted file mode 100644
index e3bd983d3661..000000000000
--- a/arch/arm64/include/asm/cputable.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * arch/arm64/include/asm/cputable.h
- *
- * Copyright (C) 2012 ARM Ltd.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-#ifndef __ASM_CPUTABLE_H
-#define __ASM_CPUTABLE_H
-
-struct cpu_info {
- unsigned int cpu_id_val;
- unsigned int cpu_id_mask;
- const char *cpu_name;
- unsigned long (*cpu_setup)(void);
-};
-
-extern struct cpu_info *lookup_processor_type(unsigned int);
-
-#endif
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 5ee07eee80c2..d5e70747c7a2 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -12,7 +12,7 @@ CFLAGS_REMOVE_insn.o = -pg
CFLAGS_REMOVE_return_address.o = -pg
# Object file lists.
-arm64-obj-y := cputable.o debug-monitors.o entry.o irq.o fpsimd.o \
+arm64-obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
entry-fpsimd.o process.o ptrace.o setup.o signal.o \
sys.o stacktrace.o time.o traps.o io.o vdso.o \
hyp-stub.o psci.o psci-call.o cpu_ops.o insn.o \
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index f7fa65d4c352..14dd3d1afa57 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -24,7 +24,6 @@
#include <linux/kvm_host.h>
#include <asm/thread_info.h>
#include <asm/memory.h>
-#include <asm/cputable.h>
#include <asm/smp_plat.h>
#include <asm/suspend.h>
#include <asm/vdso_datapage.h>
@@ -71,9 +70,6 @@ int main(void)
BLANK();
DEFINE(PAGE_SZ, PAGE_SIZE);
BLANK();
- DEFINE(CPU_INFO_SZ, sizeof(struct cpu_info));
- DEFINE(CPU_INFO_SETUP, offsetof(struct cpu_info, cpu_setup));
- BLANK();
DEFINE(DMA_BIDIRECTIONAL, DMA_BIDIRECTIONAL);
DEFINE(DMA_TO_DEVICE, DMA_TO_DEVICE);
DEFINE(DMA_FROM_DEVICE, DMA_FROM_DEVICE);
diff --git a/arch/arm64/kernel/cputable.c b/arch/arm64/kernel/cputable.c
deleted file mode 100644
index fd3993cb060f..000000000000
--- a/arch/arm64/kernel/cputable.c
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * arch/arm64/kernel/cputable.c
- *
- * Copyright (C) 2012 ARM Ltd.
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include <linux/init.h>
-
-#include <asm/cputable.h>
-
-extern unsigned long __cpu_setup(void);
-
-struct cpu_info cpu_table[] = {
- {
- .cpu_id_val = 0x000f0000,
- .cpu_id_mask = 0x000f0000,
- .cpu_name = "AArch64 Processor",
- .cpu_setup = __cpu_setup,
- },
- { /* Empty */ },
-};
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 07f930540f4a..c3631ec8fe4b 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -245,22 +245,12 @@ ENTRY(stext)
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
- bl lookup_processor_type
- mov x23, x0 // x23=current cpu_table
- /*
- * __error_p may end up out of range for cbz if text areas are
- * aligned up to section sizes.
- */
- cbnz x23, 1f // invalid processor (x23=0)?
- b __error_p
-1:
+
bl __vet_fdt
bl __create_page_tables // x25=TTBR0, x26=TTBR1
/*
- * The following calls CPU specific code in a position independent
- * manner. See arch/arm64/mm/proc.S for details. x23 = base of
- * cpu_info structure selected by lookup_processor_type above.
+ * The following calls CPU setup code, see arch/arm64/mm/proc.S for
+ * details.
* On return, the CPU will be ready for the MMU to be turned on and
* the TCR will have been set.
*/
@@ -268,9 +258,7 @@ ENTRY(stext)
// MMU has been enabled
adrp lr, __enable_mmu // return (PIC) address
add lr, lr, #:lo12:__enable_mmu
- ldr x12, [x23, #CPU_INFO_SETUP]
- add x12, x12, x28 // __virt_to_phys
- br x12 // initialise processor
+ b __cpu_setup // initialise processor
ENDPROC(stext)
/*
@@ -634,15 +622,9 @@ ENTRY(secondary_startup)
* Common entry point for secondary CPUs.
*/
mrs x22, midr_el1 // x22=cpuid
- mov x0, x22
- bl lookup_processor_type
- mov x23, x0 // x23=current cpu_table
- cbz x23, __error_p // invalid processor (x23=0)?
pgtbl x25, x26, x28 // x25=TTBR0, x26=TTBR1
- ldr x12, [x23, #CPU_INFO_SETUP]
- add x12, x12, x28 // __virt_to_phys
- blr x12 // initialise processor
+ bl __cpu_setup // initialise processor
ldr x21, =secondary_data
ldr x27, =__secondary_switched // address to jump to after enabling the MMU
@@ -708,51 +690,3 @@ ENDPROC(__calc_phys_offset)
.align 3
1: .quad .
.quad PAGE_OFFSET
-
-/*
- * Exception handling. Something went wrong and we can't proceed. We ought to
- * tell the user, but since we don't have any guarantee that we're even
- * running on the right architecture, we do virtually nothing.
- */
-__error_p:
-ENDPROC(__error_p)
-
-__error:
-1: nop
- b 1b
-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
-1:
- ldp w5, w6, [x3] // load cpu_id_val and cpu_id_mask
- cbz w5, 2f // end of list?
- and w6, w6, w0
- cmp w5, w6
- b.eq 3f
- add x3, x3, #CPU_INFO_SZ
- b 1b
-2:
- mov x3, #0 // unknown processor
-3:
- 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
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index e8420f635bd4..a19eae29497c 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -50,7 +50,6 @@
#include <asm/cpu.h>
#include <asm/cputype.h>
#include <asm/elf.h>
-#include <asm/cputable.h>
#include <asm/cpufeature.h>
#include <asm/cpu_ops.h>
#include <asm/sections.h>
@@ -83,7 +82,6 @@ unsigned int compat_elf_hwcap2 __read_mostly;
DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
-static const char *cpu_name;
phys_addr_t __fdt_pointer __initdata;
/*
@@ -209,22 +207,12 @@ static void __init smp_build_mpidr_hash(void)
static void __init setup_processor(void)
{
- struct cpu_info *cpu_info;
u64 features, block;
u32 cwg;
int cls;
- cpu_info = lookup_processor_type(read_cpuid_id());
- if (!cpu_info) {
- printk("CPU configuration botched (ID %08x), unable to continue.\n",
- read_cpuid_id());
- while (1);
- }
-
- cpu_name = cpu_info->cpu_name;
-
- printk("CPU: %s [%08x] revision %d\n",
- cpu_name, read_cpuid_id(), read_cpuid_id() & 15);
+ printk("CPU: AArch64 Processor [%08x] revision %d\n",
+ read_cpuid_id(), read_cpuid_id() & 15);
sprintf(init_utsname()->machine, ELF_PLATFORM);
elf_hwcap = 0;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v5 2/8] arm64: add macros for common adrp usages
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 1/8] arm64: Get rid of struct cpu_table Ard Biesheuvel
@ 2015-03-18 14:55 ` Ard Biesheuvel
2015-03-18 17:54 ` Mark Rutland
2015-03-18 14:55 ` [PATCH v5 3/8] arm64: remove processor_id Ard Biesheuvel
` (6 subsequent siblings)
8 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 14:55 UTC (permalink / raw)
To: linux-arm-kernel
The adrp instruction is mostly used in combination with either
an add, a ldr or a str instruction with the low bits of the
referenced symbol in the 12-bit immediate of the followup
instruction.
Introduce the macros adr_l, ldr_l and str_l that encapsulate
these common patterns.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/assembler.h | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 750bac4e637e..f1804d4803fb 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -159,4 +159,33 @@ lr .req x30 // link register
orr \rd, \lbits, \hbits, lsl #32
.endm
+/*
+ * Pseudo-ops for PC-relative adr/ldr/str <reg>, <symbol> where
+ * <symbol> is within the range +/- 4 GB of the PC.
+ */
+ .macro adr_l, dst, sym, tmp=
+ .ifb \tmp
+ adrp \dst, \sym
+ add \dst, \dst, :lo12:\sym
+ .else
+ adrp \tmp, \sym
+ add \dst, \tmp, :lo12:\sym
+ .endif
+ .endm
+
+ .macro ldr_l, dst, sym, tmp=
+ .ifb \tmp
+ adrp \dst, \sym
+ ldr \dst, [\dst, :lo12:\sym]
+ .else
+ adrp \tmp, \sym
+ ldr \dst, [\tmp, :lo12:\sym]
+ .endif
+ .endm
+
+ .macro str_l, src, sym, tmp
+ adrp \tmp, \sym
+ str \src, [\tmp, :lo12:\sym]
+ .endm
+
#endif /* __ASM_ASSEMBLER_H */
--
1.8.3.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v5 3/8] arm64: remove processor_id
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 1/8] arm64: Get rid of struct cpu_table Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 2/8] arm64: add macros for common adrp usages Ard Biesheuvel
@ 2015-03-18 14:55 ` Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 4/8] arm64: remove __switch_data object from head.S Ard Biesheuvel
` (5 subsequent siblings)
8 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 14:55 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.
As the relevance of the MIDR_EL1 value of the boot CPU is debatable
anyway, especially under big.LITTLE, let's remove it before anyone
starts using it.
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/head.S | 7 +------
arch/arm64/kernel/setup.c | 3 ---
2 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index c3631ec8fe4b..c9131f94be19 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -244,7 +244,6 @@ 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
bl __vet_fdt
bl __create_page_tables // x25=TTBR0, x26=TTBR1
@@ -427,7 +426,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
@@ -445,11 +443,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
@@ -621,8 +618,6 @@ ENTRY(secondary_startup)
/*
* Common entry point for secondary CPUs.
*/
- mrs x22, midr_el1 // x22=cpuid
-
pgtbl x25, x26, x28 // x25=TTBR0, x26=TTBR1
bl __cpu_setup // initialise processor
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index a19eae29497c..6c5fb5aff325 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -62,9 +62,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] 37+ messages in thread
* [PATCH v5 4/8] arm64: remove __switch_data object from head.S
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
` (2 preceding siblings ...)
2015-03-18 14:55 ` [PATCH v5 3/8] arm64: remove processor_id Ard Biesheuvel
@ 2015-03-18 14:55 ` Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 5/8] arm64: use PC-relative reference for secondary_holding_pen_release Ard Biesheuvel
` (4 subsequent siblings)
8 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 14:55 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.
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/head.S | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index c9131f94be19..7e8e746931af 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -253,7 +253,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
@@ -420,35 +420,22 @@ __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
+ adr_l x6, __bss_start
+ adr_l x7, __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
+ adr_l sp, initial_sp, x4
+ str_l x21, __fdt_pointer, x5 // Save FDT pointer
+ str_l x24, memstart_addr, x6 // Save PHYS_OFFSET
mov x29, #0
b start_kernel
ENDPROC(__mmap_switched)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v5 5/8] arm64: use PC-relative reference for secondary_holding_pen_release
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
` (3 preceding siblings ...)
2015-03-18 14:55 ` [PATCH v5 4/8] arm64: remove __switch_data object from head.S Ard Biesheuvel
@ 2015-03-18 14:55 ` Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 6/8] arm64: merge __enable_mmu and __turn_mmu_on Ard Biesheuvel
` (3 subsequent siblings)
8 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 14:55 UTC (permalink / raw)
To: linux-arm-kernel
Replace the confusing virtual/physical address arithmetic with a simple
PC-relative reference.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/head.S | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 7e8e746931af..65c7de889c8c 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -564,10 +564,6 @@ ENTRY(__boot_cpu_mode)
.popsection
#ifdef CONFIG_SMP
- .align 3
-1: .quad .
- .quad secondary_holding_pen_release
-
/*
* This provides a "holding pen" for platforms to hold all secondary
* cores are held until we're ready for them to initialise.
@@ -579,10 +575,7 @@ ENTRY(secondary_holding_pen)
mrs x0, mpidr_el1
ldr x1, =MPIDR_HWID_BITMASK
and x0, x0, x1
- adr x1, 1b
- ldp x2, x3, [x1]
- sub x1, x1, x2
- add x3, x3, x1
+ adr_l x3, secondary_holding_pen_release
pen: ldr x4, [x3]
cmp x4, x0
b.eq secondary_startup
--
1.8.3.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v5 6/8] arm64: merge __enable_mmu and __turn_mmu_on
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
` (4 preceding siblings ...)
2015-03-18 14:55 ` [PATCH v5 5/8] arm64: use PC-relative reference for secondary_holding_pen_release Ard Biesheuvel
@ 2015-03-18 14:55 ` Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 7/8] arm64: remove __calc_phys_offset Ard Biesheuvel
` (2 subsequent siblings)
8 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 14:55 UTC (permalink / raw)
To: linux-arm-kernel
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.
Also replaces an open coded adrp/add reference to __enable_mmu with
adr_l.
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/head.S | 33 +++++++--------------------------
1 file changed, 7 insertions(+), 26 deletions(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 65c7de889c8c..9a2554558e5b 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -255,8 +255,7 @@ ENTRY(stext)
*/
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
+ adr_l lr, __enable_mmu // return (PIC) address
b __cpu_setup // initialise processor
ENDPROC(stext)
@@ -615,11 +614,12 @@ 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.
*
- * On entry to this code, x0 must contain the SCTLR_EL1 value for turning on
- * the MMU.
+ * x0 = SCTLR_EL1 value for turning on the MMU.
+ * x27 = *virtual* address to jump to upon completion
+ *
+ * other registers depend on the function called upon completion
*/
__enable_mmu:
ldr x5, =vectors
@@ -627,29 +627,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.
--
1.8.3.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v5 7/8] arm64: remove __calc_phys_offset
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
` (5 preceding siblings ...)
2015-03-18 14:55 ` [PATCH v5 6/8] arm64: merge __enable_mmu and __turn_mmu_on Ard Biesheuvel
@ 2015-03-18 14:55 ` Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol Ard Biesheuvel
2015-03-18 18:23 ` [PATCH v5 0/8] arm64: head.S cleanup Mark Rutland
8 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 14:55 UTC (permalink / raw)
To: linux-arm-kernel
This 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.
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/head.S | 47 +++++++++++------------------------------------
1 file changed, 11 insertions(+), 36 deletions(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 9a2554558e5b..a0fbd99efb89 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -36,7 +36,7 @@
#include <asm/page.h>
#include <asm/virt.h>
-#define KERNEL_RAM_VADDR (PAGE_OFFSET + TEXT_OFFSET)
+#define __PHYS_OFFSET (KERNEL_START - TEXT_OFFSET)
#if (TEXT_OFFSET & 0xfff) != 0
#error TEXT_OFFSET must be at least 4KB aligned
@@ -46,13 +46,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 +56,7 @@
#define TABLE_SHIFT PUD_SHIFT
#endif
-#define KERNEL_START KERNEL_RAM_VADDR
+#define KERNEL_START _text
#define KERNEL_END _end
/*
@@ -242,7 +235,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, __PHYS_OFFSET
bl set_cpu_boot_mode_flag
bl __vet_fdt
@@ -342,7 +335,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
/*
@@ -371,12 +365,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
/*
@@ -385,7 +377,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
@@ -537,8 +529,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
@@ -569,7 +560,6 @@ 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
bl set_cpu_boot_mode_flag
mrs x0, mpidr_el1
ldr x1, =MPIDR_HWID_BITMASK
@@ -588,7 +578,6 @@ 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
bl set_cpu_boot_mode_flag
b secondary_startup
ENDPROC(secondary_entry)
@@ -597,7 +586,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
@@ -631,18 +621,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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
` (6 preceding siblings ...)
2015-03-18 14:55 ` [PATCH v5 7/8] arm64: remove __calc_phys_offset Ard Biesheuvel
@ 2015-03-18 14:55 ` Ard Biesheuvel
2015-03-18 18:13 ` Mark Rutland
2015-03-18 18:23 ` [PATCH v5 0/8] arm64: head.S cleanup Mark Rutland
8 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 14:55 UTC (permalink / raw)
To: linux-arm-kernel
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 <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/head.S | 4 ++++
arch/arm64/kernel/setup.c | 15 +++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index a0fbd99efb89..8636c3cef006 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -233,6 +233,10 @@ section_table:
#endif
ENTRY(stext)
+ adr_l 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, __PHYS_OFFSET
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 6c5fb5aff325..2d5cae2de679 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -114,6 +114,11 @@ void __init early_print(const char *str, ...)
printk("%s", buf);
}
+/*
+ * The recorded values of x0 .. x3 upon kernel entry.
+ */
+u64 __read_mostly boot_regs[4];
+
void __init smp_setup_processor_id(void)
{
u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
@@ -387,6 +392,16 @@ void __init setup_arch(char **cmdline_p)
conswitchp = &dummy_con;
#endif
#endif
+ /*
+ * boot_regs[] is written by the boot CPU with the caches off, so we
+ * need to ensure that we read the value from main memory
+ */
+ __flush_dcache_area(boot_regs, sizeof(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");
+ }
}
static int __init arm64_device_init(void)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v5 1/8] arm64: Get rid of struct cpu_table
2015-03-18 14:55 ` [PATCH v5 1/8] arm64: Get rid of struct cpu_table Ard Biesheuvel
@ 2015-03-18 16:11 ` Mark Rutland
2015-03-23 17:11 ` Suzuki K. Poulose
1 sibling, 0 replies; 37+ messages in thread
From: Mark Rutland @ 2015-03-18 16:11 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 18, 2015 at 02:55:20PM +0000, Ard Biesheuvel wrote:
> From: Marc Zyngier <marc.zyngier@arm.com>
>
> struct cpu_table is an artifact left from the (very) early days of
> the arm64 port, and its only real use is to allow the most beautiful
> "AArch64 Processor" string to be displayed at boot time.
>
> Really? Yes, really.
>
> Let's get rid of it. In order to avoid another BogoMips-gate, the
> aforementioned string is preserved.
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> ---
> arch/arm64/include/asm/cputable.h | 30 ----------------
> arch/arm64/kernel/Makefile | 2 +-
> arch/arm64/kernel/asm-offsets.c | 4 ---
> arch/arm64/kernel/cputable.c | 33 -----------------
> arch/arm64/kernel/head.S | 76 +++------------------------------------
> arch/arm64/kernel/setup.c | 16 ++-------
> 6 files changed, 8 insertions(+), 153 deletions(-)
> delete mode 100644 arch/arm64/include/asm/cputable.h
> delete mode 100644 arch/arm64/kernel/cputable.c
>
> diff --git a/arch/arm64/include/asm/cputable.h b/arch/arm64/include/asm/cputable.h
> deleted file mode 100644
> index e3bd983d3661..000000000000
> --- a/arch/arm64/include/asm/cputable.h
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -/*
> - * arch/arm64/include/asm/cputable.h
> - *
> - * Copyright (C) 2012 ARM Ltd.
> - *
> - * This program is free software: you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program. If not, see <http://www.gnu.org/licenses/>.
> - */
> -#ifndef __ASM_CPUTABLE_H
> -#define __ASM_CPUTABLE_H
> -
> -struct cpu_info {
> - unsigned int cpu_id_val;
> - unsigned int cpu_id_mask;
> - const char *cpu_name;
> - unsigned long (*cpu_setup)(void);
> -};
> -
> -extern struct cpu_info *lookup_processor_type(unsigned int);
> -
> -#endif
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 5ee07eee80c2..d5e70747c7a2 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -12,7 +12,7 @@ CFLAGS_REMOVE_insn.o = -pg
> CFLAGS_REMOVE_return_address.o = -pg
>
> # Object file lists.
> -arm64-obj-y := cputable.o debug-monitors.o entry.o irq.o fpsimd.o \
> +arm64-obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
> entry-fpsimd.o process.o ptrace.o setup.o signal.o \
> sys.o stacktrace.o time.o traps.o io.o vdso.o \
> hyp-stub.o psci.o psci-call.o cpu_ops.o insn.o \
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index f7fa65d4c352..14dd3d1afa57 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -24,7 +24,6 @@
> #include <linux/kvm_host.h>
> #include <asm/thread_info.h>
> #include <asm/memory.h>
> -#include <asm/cputable.h>
> #include <asm/smp_plat.h>
> #include <asm/suspend.h>
> #include <asm/vdso_datapage.h>
> @@ -71,9 +70,6 @@ int main(void)
> BLANK();
> DEFINE(PAGE_SZ, PAGE_SIZE);
> BLANK();
> - DEFINE(CPU_INFO_SZ, sizeof(struct cpu_info));
> - DEFINE(CPU_INFO_SETUP, offsetof(struct cpu_info, cpu_setup));
> - BLANK();
> DEFINE(DMA_BIDIRECTIONAL, DMA_BIDIRECTIONAL);
> DEFINE(DMA_TO_DEVICE, DMA_TO_DEVICE);
> DEFINE(DMA_FROM_DEVICE, DMA_FROM_DEVICE);
> diff --git a/arch/arm64/kernel/cputable.c b/arch/arm64/kernel/cputable.c
> deleted file mode 100644
> index fd3993cb060f..000000000000
> --- a/arch/arm64/kernel/cputable.c
> +++ /dev/null
> @@ -1,33 +0,0 @@
> -/*
> - * arch/arm64/kernel/cputable.c
> - *
> - * Copyright (C) 2012 ARM Ltd.
> - *
> - * This program is free software: you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program. If not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#include <linux/init.h>
> -
> -#include <asm/cputable.h>
> -
> -extern unsigned long __cpu_setup(void);
> -
> -struct cpu_info cpu_table[] = {
> - {
> - .cpu_id_val = 0x000f0000,
> - .cpu_id_mask = 0x000f0000,
> - .cpu_name = "AArch64 Processor",
> - .cpu_setup = __cpu_setup,
> - },
> - { /* Empty */ },
> -};
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 07f930540f4a..c3631ec8fe4b 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -245,22 +245,12 @@ ENTRY(stext)
> 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
> - bl lookup_processor_type
> - mov x23, x0 // x23=current cpu_table
> - /*
> - * __error_p may end up out of range for cbz if text areas are
> - * aligned up to section sizes.
> - */
> - cbnz x23, 1f // invalid processor (x23=0)?
> - b __error_p
> -1:
> +
> bl __vet_fdt
> bl __create_page_tables // x25=TTBR0, x26=TTBR1
> /*
> - * The following calls CPU specific code in a position independent
> - * manner. See arch/arm64/mm/proc.S for details. x23 = base of
> - * cpu_info structure selected by lookup_processor_type above.
> + * The following calls CPU setup code, see arch/arm64/mm/proc.S for
> + * details.
> * On return, the CPU will be ready for the MMU to be turned on and
> * the TCR will have been set.
> */
> @@ -268,9 +258,7 @@ ENTRY(stext)
> // MMU has been enabled
> adrp lr, __enable_mmu // return (PIC) address
> add lr, lr, #:lo12:__enable_mmu
> - ldr x12, [x23, #CPU_INFO_SETUP]
> - add x12, x12, x28 // __virt_to_phys
> - br x12 // initialise processor
> + b __cpu_setup // initialise processor
> ENDPROC(stext)
>
> /*
> @@ -634,15 +622,9 @@ ENTRY(secondary_startup)
> * Common entry point for secondary CPUs.
> */
> mrs x22, midr_el1 // x22=cpuid
> - mov x0, x22
> - bl lookup_processor_type
> - mov x23, x0 // x23=current cpu_table
> - cbz x23, __error_p // invalid processor (x23=0)?
>
> pgtbl x25, x26, x28 // x25=TTBR0, x26=TTBR1
> - ldr x12, [x23, #CPU_INFO_SETUP]
> - add x12, x12, x28 // __virt_to_phys
> - blr x12 // initialise processor
> + bl __cpu_setup // initialise processor
>
> ldr x21, =secondary_data
> ldr x27, =__secondary_switched // address to jump to after enabling the MMU
> @@ -708,51 +690,3 @@ ENDPROC(__calc_phys_offset)
> .align 3
> 1: .quad .
> .quad PAGE_OFFSET
> -
> -/*
> - * Exception handling. Something went wrong and we can't proceed. We ought to
> - * tell the user, but since we don't have any guarantee that we're even
> - * running on the right architecture, we do virtually nothing.
> - */
> -__error_p:
> -ENDPROC(__error_p)
> -
> -__error:
> -1: nop
> - b 1b
> -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
> -1:
> - ldp w5, w6, [x3] // load cpu_id_val and cpu_id_mask
> - cbz w5, 2f // end of list?
> - and w6, w6, w0
> - cmp w5, w6
> - b.eq 3f
> - add x3, x3, #CPU_INFO_SZ
> - b 1b
> -2:
> - mov x3, #0 // unknown processor
> -3:
> - 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
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index e8420f635bd4..a19eae29497c 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -50,7 +50,6 @@
> #include <asm/cpu.h>
> #include <asm/cputype.h>
> #include <asm/elf.h>
> -#include <asm/cputable.h>
> #include <asm/cpufeature.h>
> #include <asm/cpu_ops.h>
> #include <asm/sections.h>
> @@ -83,7 +82,6 @@ unsigned int compat_elf_hwcap2 __read_mostly;
>
> DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
>
> -static const char *cpu_name;
> phys_addr_t __fdt_pointer __initdata;
>
> /*
> @@ -209,22 +207,12 @@ static void __init smp_build_mpidr_hash(void)
>
> static void __init setup_processor(void)
> {
> - struct cpu_info *cpu_info;
> u64 features, block;
> u32 cwg;
> int cls;
>
> - cpu_info = lookup_processor_type(read_cpuid_id());
> - if (!cpu_info) {
> - printk("CPU configuration botched (ID %08x), unable to continue.\n",
> - read_cpuid_id());
> - while (1);
> - }
> -
> - cpu_name = cpu_info->cpu_name;
> -
> - printk("CPU: %s [%08x] revision %d\n",
> - cpu_name, read_cpuid_id(), read_cpuid_id() & 15);
> + printk("CPU: AArch64 Processor [%08x] revision %d\n",
> + read_cpuid_id(), read_cpuid_id() & 15);
>
> sprintf(init_utsname()->machine, ELF_PLATFORM);
> elf_hwcap = 0;
> --
> 1.8.3.2
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 2/8] arm64: add macros for common adrp usages
2015-03-18 14:55 ` [PATCH v5 2/8] arm64: add macros for common adrp usages Ard Biesheuvel
@ 2015-03-18 17:54 ` Mark Rutland
2015-03-18 17:56 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2015-03-18 17:54 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 18, 2015 at 02:55:21PM +0000, Ard Biesheuvel wrote:
> The adrp instruction is mostly used in combination with either
> an add, a ldr or a str instruction with the low bits of the
> referenced symbol in the 12-bit immediate of the followup
> instruction.
>
> Introduce the macros adr_l, ldr_l and str_l that encapsulate
> these common patterns.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/include/asm/assembler.h | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index 750bac4e637e..f1804d4803fb 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -159,4 +159,33 @@ lr .req x30 // link register
> orr \rd, \lbits, \hbits, lsl #32
> .endm
>
> +/*
> + * Pseudo-ops for PC-relative adr/ldr/str <reg>, <symbol> where
> + * <symbol> is within the range +/- 4 GB of the PC.
> + */
It would be nice to point out that tmp for adr_l and ldr_l is only
necesssary when loading a value into the SP.
Otherwise this looks fine.
Mark.
> + .macro adr_l, dst, sym, tmp=
> + .ifb \tmp
> + adrp \dst, \sym
> + add \dst, \dst, :lo12:\sym
> + .else
> + adrp \tmp, \sym
> + add \dst, \tmp, :lo12:\sym
> + .endif
> + .endm
> +
> + .macro ldr_l, dst, sym, tmp=
> + .ifb \tmp
> + adrp \dst, \sym
> + ldr \dst, [\dst, :lo12:\sym]
> + .else
> + adrp \tmp, \sym
> + ldr \dst, [\tmp, :lo12:\sym]
> + .endif
> + .endm
> +
> + .macro str_l, src, sym, tmp
> + adrp \tmp, \sym
> + str \src, [\tmp, :lo12:\sym]
> + .endm
> +
> #endif /* __ASM_ASSEMBLER_H */
> --
> 1.8.3.2
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 2/8] arm64: add macros for common adrp usages
2015-03-18 17:54 ` Mark Rutland
@ 2015-03-18 17:56 ` Ard Biesheuvel
2015-03-18 18:05 ` Mark Rutland
0 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 17:56 UTC (permalink / raw)
To: linux-arm-kernel
On 18 March 2015 at 18:54, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Mar 18, 2015 at 02:55:21PM +0000, Ard Biesheuvel wrote:
>> The adrp instruction is mostly used in combination with either
>> an add, a ldr or a str instruction with the low bits of the
>> referenced symbol in the 12-bit immediate of the followup
>> instruction.
>>
>> Introduce the macros adr_l, ldr_l and str_l that encapsulate
>> these common patterns.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> arch/arm64/include/asm/assembler.h | 29 +++++++++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
>> index 750bac4e637e..f1804d4803fb 100644
>> --- a/arch/arm64/include/asm/assembler.h
>> +++ b/arch/arm64/include/asm/assembler.h
>> @@ -159,4 +159,33 @@ lr .req x30 // link register
>> orr \rd, \lbits, \hbits, lsl #32
>> .endm
>>
>> +/*
>> + * Pseudo-ops for PC-relative adr/ldr/str <reg>, <symbol> where
>> + * <symbol> is within the range +/- 4 GB of the PC.
>> + */
>
> It would be nice to point out that tmp for adr_l and ldr_l is only
> necesssary when loading a value into the SP.
>
For adr_l, it is for the sp.
For ldr_l, it is primarily for being able to load w registers as well.
> Otherwise this looks fine.
>
> Mark.
>
>> + .macro adr_l, dst, sym, tmp=
>> + .ifb \tmp
>> + adrp \dst, \sym
>> + add \dst, \dst, :lo12:\sym
>> + .else
>> + adrp \tmp, \sym
>> + add \dst, \tmp, :lo12:\sym
>> + .endif
>> + .endm
>> +
>> + .macro ldr_l, dst, sym, tmp=
>> + .ifb \tmp
>> + adrp \dst, \sym
>> + ldr \dst, [\dst, :lo12:\sym]
>> + .else
>> + adrp \tmp, \sym
>> + ldr \dst, [\tmp, :lo12:\sym]
>> + .endif
>> + .endm
>> +
>> + .macro str_l, src, sym, tmp
>> + adrp \tmp, \sym
>> + str \src, [\tmp, :lo12:\sym]
>> + .endm
>> +
>> #endif /* __ASM_ASSEMBLER_H */
>> --
>> 1.8.3.2
>>
>>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 2/8] arm64: add macros for common adrp usages
2015-03-18 17:56 ` Ard Biesheuvel
@ 2015-03-18 18:05 ` Mark Rutland
2015-03-18 18:06 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2015-03-18 18:05 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 18, 2015 at 05:56:11PM +0000, Ard Biesheuvel wrote:
> On 18 March 2015 at 18:54, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Mar 18, 2015 at 02:55:21PM +0000, Ard Biesheuvel wrote:
> >> The adrp instruction is mostly used in combination with either
> >> an add, a ldr or a str instruction with the low bits of the
> >> referenced symbol in the 12-bit immediate of the followup
> >> instruction.
> >>
> >> Introduce the macros adr_l, ldr_l and str_l that encapsulate
> >> these common patterns.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >> arch/arm64/include/asm/assembler.h | 29 +++++++++++++++++++++++++++++
> >> 1 file changed, 29 insertions(+)
> >>
> >> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> >> index 750bac4e637e..f1804d4803fb 100644
> >> --- a/arch/arm64/include/asm/assembler.h
> >> +++ b/arch/arm64/include/asm/assembler.h
> >> @@ -159,4 +159,33 @@ lr .req x30 // link register
> >> orr \rd, \lbits, \hbits, lsl #32
> >> .endm
> >>
> >> +/*
> >> + * Pseudo-ops for PC-relative adr/ldr/str <reg>, <symbol> where
> >> + * <symbol> is within the range +/- 4 GB of the PC.
> >> + */
> >
> > It would be nice to point out that tmp for adr_l and ldr_l is only
> > necesssary when loading a value into the SP.
> >
>
> For adr_l, it is for the sp.
> For ldr_l, it is primarily for being able to load w registers as well.
Ah, I hadn't considered that. Are you happy to add something to the
comment block mentioning those cases?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 2/8] arm64: add macros for common adrp usages
2015-03-18 18:05 ` Mark Rutland
@ 2015-03-18 18:06 ` Ard Biesheuvel
0 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 18:06 UTC (permalink / raw)
To: linux-arm-kernel
On 18 March 2015 at 19:05, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Mar 18, 2015 at 05:56:11PM +0000, Ard Biesheuvel wrote:
>> On 18 March 2015 at 18:54, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Wed, Mar 18, 2015 at 02:55:21PM +0000, Ard Biesheuvel wrote:
>> >> The adrp instruction is mostly used in combination with either
>> >> an add, a ldr or a str instruction with the low bits of the
>> >> referenced symbol in the 12-bit immediate of the followup
>> >> instruction.
>> >>
>> >> Introduce the macros adr_l, ldr_l and str_l that encapsulate
>> >> these common patterns.
>> >>
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >> arch/arm64/include/asm/assembler.h | 29 +++++++++++++++++++++++++++++
>> >> 1 file changed, 29 insertions(+)
>> >>
>> >> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
>> >> index 750bac4e637e..f1804d4803fb 100644
>> >> --- a/arch/arm64/include/asm/assembler.h
>> >> +++ b/arch/arm64/include/asm/assembler.h
>> >> @@ -159,4 +159,33 @@ lr .req x30 // link register
>> >> orr \rd, \lbits, \hbits, lsl #32
>> >> .endm
>> >>
>> >> +/*
>> >> + * Pseudo-ops for PC-relative adr/ldr/str <reg>, <symbol> where
>> >> + * <symbol> is within the range +/- 4 GB of the PC.
>> >> + */
>> >
>> > It would be nice to point out that tmp for adr_l and ldr_l is only
>> > necesssary when loading a value into the SP.
>> >
>>
>> For adr_l, it is for the sp.
>> For ldr_l, it is primarily for being able to load w registers as well.
>
> Ah, I hadn't considered that. Are you happy to add something to the
> comment block mentioning those cases?
>
Sure, no problem
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-18 14:55 ` [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol Ard Biesheuvel
@ 2015-03-18 18:13 ` Mark Rutland
2015-03-18 18:16 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2015-03-18 18:13 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 18, 2015 at 02:55:27PM +0000, Ard Biesheuvel wrote:
> 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 <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/kernel/head.S | 4 ++++
> arch/arm64/kernel/setup.c | 15 +++++++++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index a0fbd99efb89..8636c3cef006 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -233,6 +233,10 @@ section_table:
> #endif
>
> ENTRY(stext)
> + adr_l x8, boot_regs // record the contents of
> + stp x0, x1, [x8] // x0 .. x3 at kernel entry
> + stp x2, x3, [x8, #16]
I think we should have a dc ivac here as we do for
set_cpu_boot_mode_flag.
That avoids a potential issue with boot_regs sharing a cacheline with
data we write with the MMU on -- using __flush_dcache_area will result
in a civac, so we could write back dirty data atop of the boot_regs if
there were clean entries in the cache when we did the non-cacheable
write.
Mark.
> +
> mov x21, x0 // x21=FDT
> bl el2_setup // Drop to EL1, w20=cpu_boot_mode
> adrp x24, __PHYS_OFFSET
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 6c5fb5aff325..2d5cae2de679 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -114,6 +114,11 @@ void __init early_print(const char *str, ...)
> printk("%s", buf);
> }
>
> +/*
> + * The recorded values of x0 .. x3 upon kernel entry.
> + */
> +u64 __read_mostly boot_regs[4];
> +
> void __init smp_setup_processor_id(void)
> {
> u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
> @@ -387,6 +392,16 @@ void __init setup_arch(char **cmdline_p)
> conswitchp = &dummy_con;
> #endif
> #endif
> + /*
> + * boot_regs[] is written by the boot CPU with the caches off, so we
> + * need to ensure that we read the value from main memory
> + */
> + __flush_dcache_area(boot_regs, sizeof(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");
> + }
> }
>
> static int __init arm64_device_init(void)
> --
> 1.8.3.2
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-18 18:13 ` Mark Rutland
@ 2015-03-18 18:16 ` Ard Biesheuvel
2015-03-18 18:46 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 18:16 UTC (permalink / raw)
To: linux-arm-kernel
On 18 March 2015 at 19:13, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Mar 18, 2015 at 02:55:27PM +0000, Ard Biesheuvel wrote:
>> 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 <ard.biesheuvel@linaro.org>
>> ---
>> arch/arm64/kernel/head.S | 4 ++++
>> arch/arm64/kernel/setup.c | 15 +++++++++++++++
>> 2 files changed, 19 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index a0fbd99efb89..8636c3cef006 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -233,6 +233,10 @@ section_table:
>> #endif
>>
>> ENTRY(stext)
>> + adr_l x8, boot_regs // record the contents of
>> + stp x0, x1, [x8] // x0 .. x3 at kernel entry
>> + stp x2, x3, [x8, #16]
>
> I think we should have a dc ivac here as we do for
> set_cpu_boot_mode_flag.
>
> That avoids a potential issue with boot_regs sharing a cacheline with
> data we write with the MMU on -- using __flush_dcache_area will result
> in a civac, so we could write back dirty data atop of the boot_regs if
> there were clean entries in the cache when we did the non-cacheable
> write.
>
Hmm, I wondered about that.
Could we instead just make it u64 __initconst boot_regs[] in setup.c ?
>> +
>> mov x21, x0 // x21=FDT
>> bl el2_setup // Drop to EL1, w20=cpu_boot_mode
>> adrp x24, __PHYS_OFFSET
>> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
>> index 6c5fb5aff325..2d5cae2de679 100644
>> --- a/arch/arm64/kernel/setup.c
>> +++ b/arch/arm64/kernel/setup.c
>> @@ -114,6 +114,11 @@ void __init early_print(const char *str, ...)
>> printk("%s", buf);
>> }
>>
>> +/*
>> + * The recorded values of x0 .. x3 upon kernel entry.
>> + */
>> +u64 __read_mostly boot_regs[4];
>> +
>> void __init smp_setup_processor_id(void)
>> {
>> u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
>> @@ -387,6 +392,16 @@ void __init setup_arch(char **cmdline_p)
>> conswitchp = &dummy_con;
>> #endif
>> #endif
>> + /*
>> + * boot_regs[] is written by the boot CPU with the caches off, so we
>> + * need to ensure that we read the value from main memory
>> + */
>> + __flush_dcache_area(boot_regs, sizeof(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");
>> + }
>> }
>>
>> static int __init arm64_device_init(void)
>> --
>> 1.8.3.2
>>
>>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 0/8] arm64: head.S cleanup
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
` (7 preceding siblings ...)
2015-03-18 14:55 ` [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol Ard Biesheuvel
@ 2015-03-18 18:23 ` Mark Rutland
2015-03-18 18:28 ` Ard Biesheuvel
8 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2015-03-18 18:23 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 18, 2015 at 02:55:19PM +0000, Ard Biesheuvel wrote:
> This some janitorial work on head.S, just stuff I noticed when making
> changes to it for other reasons.
>
> This still needs acks on patches #2, #5 and #8
I've given each patch of the series series a spin on Juno (using
EFI+PSCI) and a model (using the bootwwapper + spin-table). For patches
1-7 the code looks sane, I see no build regressions on defconfig, and
everything works, so for those feel free to add:
Tested-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
I have some comments against patch 8 that will need to be fixed up, so
I'll provide tags for that separately.
Mark.
>
> Changes since v4:
> - included Marc Zyngier's struct cpu_table removal patch
> - dropped __lookup_processor_type_data patch now that Marc has nuked the
> whole thing
> - added patch to merge __enable_mmu() and __turn_mmu_on() into a single
> function, as there is no need for funky trampoline stuff when you have
> two non-overlapping TTBRs
> - added patch to remove __calc_phys_offset and use PC-relative references or
> absolute (linker generated) references as appropriate
> - added patch to complain when x1/x2/x3 are non-zero, which helps ensure that
> we will ever be able to use them for anything
> - added R-b's
>
> Changes since v3:
> - added similar patch for secondary_holding_pen_release
> - fixed bug in patch #1 (ldr_l)
>
> Changes since v2:
> - added separate patch to add macros for adrp/add, adrp/ldr and adrp/str
> - added R-b's
>
> Ard Biesheuvel (7):
> arm64: add macros for common adrp usages
> arm64: remove processor_id
> arm64: remove __switch_data object from head.S
> arm64: use PC-relative reference for secondary_holding_pen_release
> 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
>
> Marc Zyngier (1):
> arm64: Get rid of struct cpu_table
>
> arch/arm64/include/asm/assembler.h | 29 ++++++
> arch/arm64/include/asm/cputable.h | 30 ------
> arch/arm64/kernel/Makefile | 2 +-
> arch/arm64/kernel/asm-offsets.c | 4 -
> arch/arm64/kernel/cputable.c | 33 ------
> arch/arm64/kernel/head.S | 203 +++++++------------------------------
> arch/arm64/kernel/setup.c | 34 +++----
> 7 files changed, 83 insertions(+), 252 deletions(-)
> delete mode 100644 arch/arm64/include/asm/cputable.h
> delete mode 100644 arch/arm64/kernel/cputable.c
>
> --
> 1.8.3.2
>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 0/8] arm64: head.S cleanup
2015-03-18 18:23 ` [PATCH v5 0/8] arm64: head.S cleanup Mark Rutland
@ 2015-03-18 18:28 ` Ard Biesheuvel
0 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 18:28 UTC (permalink / raw)
To: linux-arm-kernel
On 18 March 2015 at 19:23, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Mar 18, 2015 at 02:55:19PM +0000, Ard Biesheuvel wrote:
>> This some janitorial work on head.S, just stuff I noticed when making
>> changes to it for other reasons.
>>
>> This still needs acks on patches #2, #5 and #8
>
> I've given each patch of the series series a spin on Juno (using
> EFI+PSCI) and a model (using the bootwwapper + spin-table). For patches
> 1-7 the code looks sane, I see no build regressions on defconfig, and
> everything works, so for those feel free to add:
>
> Tested-by: Mark Rutland <mark.rutland@arm.com>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>
Thanks!
>>
>> Changes since v4:
>> - included Marc Zyngier's struct cpu_table removal patch
>> - dropped __lookup_processor_type_data patch now that Marc has nuked the
>> whole thing
>> - added patch to merge __enable_mmu() and __turn_mmu_on() into a single
>> function, as there is no need for funky trampoline stuff when you have
>> two non-overlapping TTBRs
>> - added patch to remove __calc_phys_offset and use PC-relative references or
>> absolute (linker generated) references as appropriate
>> - added patch to complain when x1/x2/x3 are non-zero, which helps ensure that
>> we will ever be able to use them for anything
>> - added R-b's
>>
>> Changes since v3:
>> - added similar patch for secondary_holding_pen_release
>> - fixed bug in patch #1 (ldr_l)
>>
>> Changes since v2:
>> - added separate patch to add macros for adrp/add, adrp/ldr and adrp/str
>> - added R-b's
>>
>> Ard Biesheuvel (7):
>> arm64: add macros for common adrp usages
>> arm64: remove processor_id
>> arm64: remove __switch_data object from head.S
>> arm64: use PC-relative reference for secondary_holding_pen_release
>> 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
>>
>> Marc Zyngier (1):
>> arm64: Get rid of struct cpu_table
>>
>> arch/arm64/include/asm/assembler.h | 29 ++++++
>> arch/arm64/include/asm/cputable.h | 30 ------
>> arch/arm64/kernel/Makefile | 2 +-
>> arch/arm64/kernel/asm-offsets.c | 4 -
>> arch/arm64/kernel/cputable.c | 33 ------
>> arch/arm64/kernel/head.S | 203 +++++++------------------------------
>> arch/arm64/kernel/setup.c | 34 +++----
>> 7 files changed, 83 insertions(+), 252 deletions(-)
>> delete mode 100644 arch/arm64/include/asm/cputable.h
>> delete mode 100644 arch/arm64/kernel/cputable.c
>>
>> --
>> 1.8.3.2
>>
>>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-18 18:16 ` Ard Biesheuvel
@ 2015-03-18 18:46 ` Ard Biesheuvel
2015-03-18 18:57 ` Mark Rutland
0 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 18:46 UTC (permalink / raw)
To: linux-arm-kernel
On 18 March 2015 at 19:16, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 18 March 2015 at 19:13, Mark Rutland <mark.rutland@arm.com> wrote:
>> On Wed, Mar 18, 2015 at 02:55:27PM +0000, Ard Biesheuvel wrote:
>>> 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 <ard.biesheuvel@linaro.org>
>>> ---
>>> arch/arm64/kernel/head.S | 4 ++++
>>> arch/arm64/kernel/setup.c | 15 +++++++++++++++
>>> 2 files changed, 19 insertions(+)
>>>
>>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>>> index a0fbd99efb89..8636c3cef006 100644
>>> --- a/arch/arm64/kernel/head.S
>>> +++ b/arch/arm64/kernel/head.S
>>> @@ -233,6 +233,10 @@ section_table:
>>> #endif
>>>
>>> ENTRY(stext)
>>> + adr_l x8, boot_regs // record the contents of
>>> + stp x0, x1, [x8] // x0 .. x3 at kernel entry
>>> + stp x2, x3, [x8, #16]
>>
>> I think we should have a dc ivac here as we do for
>> set_cpu_boot_mode_flag.
>>
>> That avoids a potential issue with boot_regs sharing a cacheline with
>> data we write with the MMU on -- using __flush_dcache_area will result
>> in a civac, so we could write back dirty data atop of the boot_regs if
>> there were clean entries in the cache when we did the non-cacheable
>> write.
>>
>
> Hmm, I wondered about that.
>
> Could we instead just make it u64 __initconst boot_regs[] in setup.c ?
>
Never mind, it's easier just to do the invalidate right after, and I
can drop the flush before the access.
>>> +
>>> mov x21, x0 // x21=FDT
>>> bl el2_setup // Drop to EL1, w20=cpu_boot_mode
>>> adrp x24, __PHYS_OFFSET
>>> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
>>> index 6c5fb5aff325..2d5cae2de679 100644
>>> --- a/arch/arm64/kernel/setup.c
>>> +++ b/arch/arm64/kernel/setup.c
>>> @@ -114,6 +114,11 @@ void __init early_print(const char *str, ...)
>>> printk("%s", buf);
>>> }
>>>
>>> +/*
>>> + * The recorded values of x0 .. x3 upon kernel entry.
>>> + */
>>> +u64 __read_mostly boot_regs[4];
>>> +
>>> void __init smp_setup_processor_id(void)
>>> {
>>> u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
>>> @@ -387,6 +392,16 @@ void __init setup_arch(char **cmdline_p)
>>> conswitchp = &dummy_con;
>>> #endif
>>> #endif
>>> + /*
>>> + * boot_regs[] is written by the boot CPU with the caches off, so we
>>> + * need to ensure that we read the value from main memory
>>> + */
>>> + __flush_dcache_area(boot_regs, sizeof(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");
>>> + }
>>> }
>>>
>>> static int __init arm64_device_init(void)
>>> --
>>> 1.8.3.2
>>>
>>>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-18 18:46 ` Ard Biesheuvel
@ 2015-03-18 18:57 ` Mark Rutland
2015-03-18 19:55 ` Ard Biesheuvel
2015-03-18 22:26 ` [PATCH v5 8/8] " Peter Maydell
0 siblings, 2 replies; 37+ messages in thread
From: Mark Rutland @ 2015-03-18 18:57 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 18, 2015 at 06:46:11PM +0000, Ard Biesheuvel wrote:
> On 18 March 2015 at 19:16, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 18 March 2015 at 19:13, Mark Rutland <mark.rutland@arm.com> wrote:
> >> On Wed, Mar 18, 2015 at 02:55:27PM +0000, Ard Biesheuvel wrote:
> >>> 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 <ard.biesheuvel@linaro.org>
> >>> ---
> >>> arch/arm64/kernel/head.S | 4 ++++
> >>> arch/arm64/kernel/setup.c | 15 +++++++++++++++
> >>> 2 files changed, 19 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> >>> index a0fbd99efb89..8636c3cef006 100644
> >>> --- a/arch/arm64/kernel/head.S
> >>> +++ b/arch/arm64/kernel/head.S
> >>> @@ -233,6 +233,10 @@ section_table:
> >>> #endif
> >>>
> >>> ENTRY(stext)
> >>> + adr_l x8, boot_regs // record the contents of
> >>> + stp x0, x1, [x8] // x0 .. x3 at kernel entry
> >>> + stp x2, x3, [x8, #16]
> >>
> >> I think we should have a dc ivac here as we do for
> >> set_cpu_boot_mode_flag.
> >>
> >> That avoids a potential issue with boot_regs sharing a cacheline with
> >> data we write with the MMU on -- using __flush_dcache_area will result
> >> in a civac, so we could write back dirty data atop of the boot_regs if
> >> there were clean entries in the cache when we did the non-cacheable
> >> write.
> >>
> >
> > Hmm, I wondered about that.
> >
> > Could we instead just make it u64 __initconst boot_regs[] in setup.c ?
> >
>
> Never mind, it's easier just to do the invalidate right after, and I
> can drop the flush before the access.
Yup.
Annoyingly the minimum cache line size seems to be a word (given the
defnition of CTR.DminLine), which means you need a few dc ivac
instructions to be architecturally correct.
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-18 18:57 ` Mark Rutland
@ 2015-03-18 19:55 ` Ard Biesheuvel
2015-03-18 20:24 ` Mark Rutland
2015-03-18 22:26 ` [PATCH v5 8/8] " Peter Maydell
1 sibling, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-18 19:55 UTC (permalink / raw)
To: linux-arm-kernel
On 18 March 2015 at 19:57, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Mar 18, 2015 at 06:46:11PM +0000, Ard Biesheuvel wrote:
>> On 18 March 2015 at 19:16, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > On 18 March 2015 at 19:13, Mark Rutland <mark.rutland@arm.com> wrote:
>> >> On Wed, Mar 18, 2015 at 02:55:27PM +0000, Ard Biesheuvel wrote:
>> >>> 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 <ard.biesheuvel@linaro.org>
>> >>> ---
>> >>> arch/arm64/kernel/head.S | 4 ++++
>> >>> arch/arm64/kernel/setup.c | 15 +++++++++++++++
>> >>> 2 files changed, 19 insertions(+)
>> >>>
>> >>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> >>> index a0fbd99efb89..8636c3cef006 100644
>> >>> --- a/arch/arm64/kernel/head.S
>> >>> +++ b/arch/arm64/kernel/head.S
>> >>> @@ -233,6 +233,10 @@ section_table:
>> >>> #endif
>> >>>
>> >>> ENTRY(stext)
>> >>> + adr_l x8, boot_regs // record the contents of
>> >>> + stp x0, x1, [x8] // x0 .. x3 at kernel entry
>> >>> + stp x2, x3, [x8, #16]
>> >>
>> >> I think we should have a dc ivac here as we do for
>> >> set_cpu_boot_mode_flag.
>> >>
>> >> That avoids a potential issue with boot_regs sharing a cacheline with
>> >> data we write with the MMU on -- using __flush_dcache_area will result
>> >> in a civac, so we could write back dirty data atop of the boot_regs if
>> >> there were clean entries in the cache when we did the non-cacheable
>> >> write.
>> >>
>> >
>> > Hmm, I wondered about that.
>> >
>> > Could we instead just make it u64 __initconst boot_regs[] in setup.c ?
>> >
>>
>> Never mind, it's easier just to do the invalidate right after, and I
>> can drop the flush before the access.
>
> Yup.
>
> Annoyingly the minimum cache line size seems to be a word (given the
> defnition of CTR.DminLine), which means you need a few dc ivac
> instructions to be architecturally correct.
>
But that applies to cpu_boot_mode as well then?
I will add a call to __inval_cache_range() right after recording the
initial values, that should do the right thing regarding llinesize
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-18 19:55 ` Ard Biesheuvel
@ 2015-03-18 20:24 ` Mark Rutland
2015-03-19 7:30 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2015-03-18 20:24 UTC (permalink / raw)
To: linux-arm-kernel
> >> >>> ENTRY(stext)
> >> >>> + adr_l x8, boot_regs // record the contents of
> >> >>> + stp x0, x1, [x8] // x0 .. x3 at kernel entry
> >> >>> + stp x2, x3, [x8, #16]
> >> >>
> >> >> I think we should have a dc ivac here as we do for
> >> >> set_cpu_boot_mode_flag.
> >> >>
> >> >> That avoids a potential issue with boot_regs sharing a cacheline with
> >> >> data we write with the MMU on -- using __flush_dcache_area will result
> >> >> in a civac, so we could write back dirty data atop of the boot_regs if
> >> >> there were clean entries in the cache when we did the non-cacheable
> >> >> write.
> >> >>
> >> >
> >> > Hmm, I wondered about that.
> >> >
> >> > Could we instead just make it u64 __initconst boot_regs[] in setup.c ?
> >> >
> >>
> >> Never mind, it's easier just to do the invalidate right after, and I
> >> can drop the flush before the access.
> >
> > Yup.
> >
> > Annoyingly the minimum cache line size seems to be a word (given the
> > defnition of CTR.DminLine), which means you need a few dc ivac
> > instructions to be architecturally correct.
> >
>
> But that applies to cpu_boot_mode as well then?
It writes a single word, so it happens to be safe.
> I will add a call to __inval_cache_range() right after recording the
> initial values, that should do the right thing regarding llinesize
That works, with one caveat: you'll need a dmb sy between the writes and
the call -- dc instructions by VA only hazard against normal cacheable
accesses, and __inval_cache_range assumes the caches are on and so
doesn't have a dmb prior to the dc instructions.
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-18 18:57 ` Mark Rutland
2015-03-18 19:55 ` Ard Biesheuvel
@ 2015-03-18 22:26 ` Peter Maydell
1 sibling, 0 replies; 37+ messages in thread
From: Peter Maydell @ 2015-03-18 22:26 UTC (permalink / raw)
To: linux-arm-kernel
On 18 March 2015 at 18:57, Mark Rutland <mark.rutland@arm.com> wrote:
> Annoyingly the minimum cache line size seems to be a word (given the
> defnition of CTR.DminLine), which means you need a few dc ivac
> instructions to be architecturally correct.
Section D3.4.1 says "For the purpose of these principles, a
cache entry covers at least 16 bytes and no more than 2KB of
contiguous address space, aligned to its size.", so (depending
on what you think the "for the purpose of these principles"
means) you may be OK there.
-- PMM
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-18 20:24 ` Mark Rutland
@ 2015-03-19 7:30 ` Ard Biesheuvel
2015-03-19 10:35 ` Mark Rutland
0 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-19 7:30 UTC (permalink / raw)
To: linux-arm-kernel
On 18 March 2015 at 21:24, Mark Rutland <mark.rutland@arm.com> wrote:
>> >> >>> ENTRY(stext)
>> >> >>> + adr_l x8, boot_regs // record the contents of
>> >> >>> + stp x0, x1, [x8] // x0 .. x3 at kernel entry
>> >> >>> + stp x2, x3, [x8, #16]
>> >> >>
>> >> >> I think we should have a dc ivac here as we do for
>> >> >> set_cpu_boot_mode_flag.
>> >> >>
>> >> >> That avoids a potential issue with boot_regs sharing a cacheline with
>> >> >> data we write with the MMU on -- using __flush_dcache_area will result
>> >> >> in a civac, so we could write back dirty data atop of the boot_regs if
>> >> >> there were clean entries in the cache when we did the non-cacheable
>> >> >> write.
>> >> >>
>> >> >
>> >> > Hmm, I wondered about that.
>> >> >
>> >> > Could we instead just make it u64 __initconst boot_regs[] in setup.c ?
>> >> >
>> >>
>> >> Never mind, it's easier just to do the invalidate right after, and I
>> >> can drop the flush before the access.
>> >
>> > Yup.
>> >
>> > Annoyingly the minimum cache line size seems to be a word (given the
>> > defnition of CTR.DminLine), which means you need a few dc ivac
>> > instructions to be architecturally correct.
>> >
>>
>> But that applies to cpu_boot_mode as well then?
>
> It writes a single word, so it happens to be safe.
>
>> I will add a call to __inval_cache_range() right after recording the
>> initial values, that should do the right thing regarding llinesize
>
> That works, with one caveat: you'll need a dmb sy between the writes and
> the call -- dc instructions by VA only hazard against normal cacheable
> accesses, and __inval_cache_range assumes the caches are on and so
> doesn't have a dmb prior to the dc instructions.
>
Does it matter at all that __inval_cache_range() will mostly end up
doing a civac for the whole array, since it uses civac not ivac for
both non-cachelined aligned ends of the region, and the typical
cacheline size is larger then the size of the array? Couldn't that
also clobber what we just wrote with a stale cacheline?
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-19 7:30 ` Ard Biesheuvel
@ 2015-03-19 10:35 ` Mark Rutland
2015-03-19 10:38 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2015-03-19 10:35 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 19, 2015 at 07:30:03AM +0000, Ard Biesheuvel wrote:
> On 18 March 2015 at 21:24, Mark Rutland <mark.rutland@arm.com> wrote:
> >> >> >>> ENTRY(stext)
> >> >> >>> + adr_l x8, boot_regs // record the contents of
> >> >> >>> + stp x0, x1, [x8] // x0 .. x3 at kernel entry
> >> >> >>> + stp x2, x3, [x8, #16]
> >> >> >>
> >> >> >> I think we should have a dc ivac here as we do for
> >> >> >> set_cpu_boot_mode_flag.
> >> >> >>
> >> >> >> That avoids a potential issue with boot_regs sharing a cacheline with
> >> >> >> data we write with the MMU on -- using __flush_dcache_area will result
> >> >> >> in a civac, so we could write back dirty data atop of the boot_regs if
> >> >> >> there were clean entries in the cache when we did the non-cacheable
> >> >> >> write.
> >> >> >>
> >> >> >
> >> >> > Hmm, I wondered about that.
> >> >> >
> >> >> > Could we instead just make it u64 __initconst boot_regs[] in setup.c ?
> >> >> >
> >> >>
> >> >> Never mind, it's easier just to do the invalidate right after, and I
> >> >> can drop the flush before the access.
> >> >
> >> > Yup.
> >> >
> >> > Annoyingly the minimum cache line size seems to be a word (given the
> >> > defnition of CTR.DminLine), which means you need a few dc ivac
> >> > instructions to be architecturally correct.
> >> >
> >>
> >> But that applies to cpu_boot_mode as well then?
> >
> > It writes a single word, so it happens to be safe.
> >
> >> I will add a call to __inval_cache_range() right after recording the
> >> initial values, that should do the right thing regarding llinesize
> >
> > That works, with one caveat: you'll need a dmb sy between the writes and
> > the call -- dc instructions by VA only hazard against normal cacheable
> > accesses, and __inval_cache_range assumes the caches are on and so
> > doesn't have a dmb prior to the dc instructions.
> >
>
> Does it matter at all that __inval_cache_range() will mostly end up
> doing a civac for the whole array, since it uses civac not ivac for
> both non-cachelined aligned ends of the region, and the typical
> cacheline size is larger then the size of the array? Couldn't that
> also clobber what we just wrote with a stale cacheline?
Yes, though only if the memory were outside the footprint of the loaded
Image (which per the boot protocol should be clean to the PoC).
So I guess we should move the boot_regs structure back into head.S so it
doesn't fall outside
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-19 10:35 ` Mark Rutland
@ 2015-03-19 10:38 ` Ard Biesheuvel
2015-03-19 10:41 ` Mark Rutland
0 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-19 10:38 UTC (permalink / raw)
To: linux-arm-kernel
On 19 March 2015 at 11:35, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Mar 19, 2015 at 07:30:03AM +0000, Ard Biesheuvel wrote:
>> On 18 March 2015 at 21:24, Mark Rutland <mark.rutland@arm.com> wrote:
>> >> >> >>> ENTRY(stext)
>> >> >> >>> + adr_l x8, boot_regs // record the contents of
>> >> >> >>> + stp x0, x1, [x8] // x0 .. x3 at kernel entry
>> >> >> >>> + stp x2, x3, [x8, #16]
>> >> >> >>
>> >> >> >> I think we should have a dc ivac here as we do for
>> >> >> >> set_cpu_boot_mode_flag.
>> >> >> >>
>> >> >> >> That avoids a potential issue with boot_regs sharing a cacheline with
>> >> >> >> data we write with the MMU on -- using __flush_dcache_area will result
>> >> >> >> in a civac, so we could write back dirty data atop of the boot_regs if
>> >> >> >> there were clean entries in the cache when we did the non-cacheable
>> >> >> >> write.
>> >> >> >>
>> >> >> >
>> >> >> > Hmm, I wondered about that.
>> >> >> >
>> >> >> > Could we instead just make it u64 __initconst boot_regs[] in setup.c ?
>> >> >> >
>> >> >>
>> >> >> Never mind, it's easier just to do the invalidate right after, and I
>> >> >> can drop the flush before the access.
>> >> >
>> >> > Yup.
>> >> >
>> >> > Annoyingly the minimum cache line size seems to be a word (given the
>> >> > defnition of CTR.DminLine), which means you need a few dc ivac
>> >> > instructions to be architecturally correct.
>> >> >
>> >>
>> >> But that applies to cpu_boot_mode as well then?
>> >
>> > It writes a single word, so it happens to be safe.
>> >
>> >> I will add a call to __inval_cache_range() right after recording the
>> >> initial values, that should do the right thing regarding llinesize
>> >
>> > That works, with one caveat: you'll need a dmb sy between the writes and
>> > the call -- dc instructions by VA only hazard against normal cacheable
>> > accesses, and __inval_cache_range assumes the caches are on and so
>> > doesn't have a dmb prior to the dc instructions.
>> >
>>
>> Does it matter at all that __inval_cache_range() will mostly end up
>> doing a civac for the whole array, since it uses civac not ivac for
>> both non-cachelined aligned ends of the region, and the typical
>> cacheline size is larger then the size of the array? Couldn't that
>> also clobber what we just wrote with a stale cacheline?
>
> Yes, though only if the memory were outside the footprint of the loaded
> Image (which per the boot protocol should be clean to the PoC).
>
> So I guess we should move the boot_regs structure back into head.S so it
> doesn't fall outside
>
OK that means .data should be fine too. __cacheline_aligned variables
are put into the .data section, so let me use that instead (.bss gets
cleared after anyway, that is why i added the __read_mostly initially)
--
Ard.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-19 10:38 ` Ard Biesheuvel
@ 2015-03-19 10:41 ` Mark Rutland
2015-03-19 11:00 ` [PATCH v3] " Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2015-03-19 10:41 UTC (permalink / raw)
To: linux-arm-kernel
> >> Does it matter at all that __inval_cache_range() will mostly end up
> >> doing a civac for the whole array, since it uses civac not ivac for
> >> both non-cachelined aligned ends of the region, and the typical
> >> cacheline size is larger then the size of the array? Couldn't that
> >> also clobber what we just wrote with a stale cacheline?
> >
> > Yes, though only if the memory were outside the footprint of the loaded
> > Image (which per the boot protocol should be clean to the PoC).
> >
> > So I guess we should move the boot_regs structure back into head.S so it
> > doesn't fall outside
> >
>
> OK that means .data should be fine too. __cacheline_aligned variables
> are put into the .data section, so let me use that instead (.bss gets
> cleared after anyway, that is why i added the __read_mostly initially)
Great. Using __cachelign_aligned sounds good to me.
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-19 10:41 ` Mark Rutland
@ 2015-03-19 11:00 ` Ard Biesheuvel
2015-03-19 13:36 ` Mark Rutland
0 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-19 11:00 UTC (permalink / raw)
To: linux-arm-kernel
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 <ard.biesheuvel@linaro.org>
---
arch/arm64/kernel/head.S | 19 ++++++++++++++++++-
arch/arm64/kernel/setup.c | 10 ++++++++++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index f5ac337f9598..1fdf42041f42 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -233,7 +233,7 @@ section_table:
#endif
ENTRY(stext)
- mov x21, x0 // x21=FDT
+ bl preserve_boot_args
bl el2_setup // Drop to EL1, w20=cpu_boot_mode
adrp x24, __PHYS_OFFSET
bl set_cpu_boot_mode_flag
@@ -253,6 +253,23 @@ ENTRY(stext)
ENDPROC(stext)
/*
+ * Preserve the arguments passed by the bootloader in x0 .. x3
+ */
+preserve_boot_args:
+ mov x21, x0 // x21=FDT
+
+ adr_l x0, boot_args // record the contents of
+ stp x21, x1, [x0] // x0 .. x3 at kernel entry
+ stp x2, x3, [x0, #16]
+
+ dmb sy // needed before dc ivac with
+ // MMU off
+
+ add x1, x0, #0x20 // 4 x 8 bytes
+ b __inval_cache_range // tail call
+ENDPROC(preserve_boot_args)
+
+/*
* Determine validity of the x21 FDT pointer.
* The dtb must be 8-byte aligned and live in the first 512M of memory.
*/
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 1783b38cf4c0..2f384019b201 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -115,6 +115,11 @@ void __init early_print(const char *str, ...)
printk("%s", buf);
}
+/*
+ * The recorded values of x0 .. x3 upon kernel entry.
+ */
+u64 __cacheline_aligned boot_args[4];
+
void __init smp_setup_processor_id(void)
{
u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
@@ -412,6 +417,11 @@ void __init setup_arch(char **cmdline_p)
conswitchp = &dummy_con;
#endif
#endif
+ if (boot_args[1] || boot_args[2] || boot_args[3]) {
+ pr_err("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n",
+ boot_args[1], boot_args[2], boot_args[3]);
+ pr_err("WARNING: your bootloader may fail to load newer kernels\n");
+ }
}
static int __init arm64_device_init(void)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-19 11:00 ` [PATCH v3] " Ard Biesheuvel
@ 2015-03-19 13:36 ` Mark Rutland
2015-03-20 11:31 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2015-03-19 13:36 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 19, 2015 at 11:00:52AM +0000, Ard Biesheuvel wrote:
> 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 <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/kernel/head.S | 19 ++++++++++++++++++-
> arch/arm64/kernel/setup.c | 10 ++++++++++
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index f5ac337f9598..1fdf42041f42 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -233,7 +233,7 @@ section_table:
> #endif
>
> ENTRY(stext)
> - mov x21, x0 // x21=FDT
> + bl preserve_boot_args
> bl el2_setup // Drop to EL1, w20=cpu_boot_mode
> adrp x24, __PHYS_OFFSET
> bl set_cpu_boot_mode_flag
> @@ -253,6 +253,23 @@ ENTRY(stext)
> ENDPROC(stext)
>
> /*
> + * Preserve the arguments passed by the bootloader in x0 .. x3
> + */
> +preserve_boot_args:
> + mov x21, x0 // x21=FDT
> +
> + adr_l x0, boot_args // record the contents of
> + stp x21, x1, [x0] // x0 .. x3 at kernel entry
> + stp x2, x3, [x0, #16]
> +
> + dmb sy // needed before dc ivac with
> + // MMU off
> +
> + add x1, x0, #0x20 // 4 x 8 bytes
> + b __inval_cache_range // tail call
> +ENDPROC(preserve_boot_args)
> +
> +/*
> * Determine validity of the x21 FDT pointer.
> * The dtb must be 8-byte aligned and live in the first 512M of memory.
> */
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 1783b38cf4c0..2f384019b201 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -115,6 +115,11 @@ void __init early_print(const char *str, ...)
> printk("%s", buf);
> }
>
> +/*
> + * The recorded values of x0 .. x3 upon kernel entry.
> + */
> +u64 __cacheline_aligned boot_args[4];
All the above looks correct to me.
> +
> void __init smp_setup_processor_id(void)
> {
> u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
> @@ -412,6 +417,11 @@ void __init setup_arch(char **cmdline_p)
> conswitchp = &dummy_con;
> #endif
> #endif
> + if (boot_args[1] || boot_args[2] || boot_args[3]) {
> + pr_err("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n",
> + boot_args[1], boot_args[2], boot_args[3]);
> + pr_err("WARNING: your bootloader may fail to load newer kernels\n");
If we ever decide to use x1-x3 for something, and try to boot an older
kernel, that warning is going to be a bit misleading. That could matter
for VMs where we're going to see old kernel images for a long time.
I would like the warning to mention that could be the case.
It would also be nice if the message were consistently spaced regardless
of the values of x1-x3, so we should zero-pad them (and as that takes a
resonable amount of space, let's give them a line each).
So could we change the warning to be something like:
pr_err("WARNING: x1-x3 nonzero in violation of boot protocol:\n"
"\tx1: %016llx\n\tx2: %016llx\n\tx3: %016llx\n"
"This indicates a broken bootloader or old kernel\n",
boot_args[1], boot_args[2], boot_args[3]);
With that,
Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-19 13:36 ` Mark Rutland
@ 2015-03-20 11:31 ` Ard Biesheuvel
2015-03-20 11:41 ` Mark Rutland
0 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-20 11:31 UTC (permalink / raw)
To: linux-arm-kernel
On 19 March 2015 at 14:36, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Mar 19, 2015 at 11:00:52AM +0000, Ard Biesheuvel wrote:
>> 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 <ard.biesheuvel@linaro.org>
>> ---
>> arch/arm64/kernel/head.S | 19 ++++++++++++++++++-
>> arch/arm64/kernel/setup.c | 10 ++++++++++
>> 2 files changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index f5ac337f9598..1fdf42041f42 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -233,7 +233,7 @@ section_table:
>> #endif
>>
>> ENTRY(stext)
>> - mov x21, x0 // x21=FDT
>> + bl preserve_boot_args
>> bl el2_setup // Drop to EL1, w20=cpu_boot_mode
>> adrp x24, __PHYS_OFFSET
>> bl set_cpu_boot_mode_flag
>> @@ -253,6 +253,23 @@ ENTRY(stext)
>> ENDPROC(stext)
>>
>> /*
>> + * Preserve the arguments passed by the bootloader in x0 .. x3
>> + */
>> +preserve_boot_args:
>> + mov x21, x0 // x21=FDT
>> +
>> + adr_l x0, boot_args // record the contents of
>> + stp x21, x1, [x0] // x0 .. x3 at kernel entry
>> + stp x2, x3, [x0, #16]
>> +
>> + dmb sy // needed before dc ivac with
>> + // MMU off
>> +
>> + add x1, x0, #0x20 // 4 x 8 bytes
>> + b __inval_cache_range // tail call
>> +ENDPROC(preserve_boot_args)
>> +
>> +/*
>> * Determine validity of the x21 FDT pointer.
>> * The dtb must be 8-byte aligned and live in the first 512M of memory.
>> */
>> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
>> index 1783b38cf4c0..2f384019b201 100644
>> --- a/arch/arm64/kernel/setup.c
>> +++ b/arch/arm64/kernel/setup.c
>> @@ -115,6 +115,11 @@ void __init early_print(const char *str, ...)
>> printk("%s", buf);
>> }
>>
>> +/*
>> + * The recorded values of x0 .. x3 upon kernel entry.
>> + */
>> +u64 __cacheline_aligned boot_args[4];
>
> All the above looks correct to me.
>
>> +
>> void __init smp_setup_processor_id(void)
>> {
>> u64 mpidr = read_cpuid_mpidr() & MPIDR_HWID_BITMASK;
>> @@ -412,6 +417,11 @@ void __init setup_arch(char **cmdline_p)
>> conswitchp = &dummy_con;
>> #endif
>> #endif
>> + if (boot_args[1] || boot_args[2] || boot_args[3]) {
>> + pr_err("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n",
>> + boot_args[1], boot_args[2], boot_args[3]);
>> + pr_err("WARNING: your bootloader may fail to load newer kernels\n");
>
> If we ever decide to use x1-x3 for something, and try to boot an older
> kernel, that warning is going to be a bit misleading. That could matter
> for VMs where we're going to see old kernel images for a long time.
>
> I would like the warning to mention that could be the case.
>
> It would also be nice if the message were consistently spaced regardless
> of the values of x1-x3, so we should zero-pad them (and as that takes a
> resonable amount of space, let's give them a line each).
>
> So could we change the warning to be something like:
>
> pr_err("WARNING: x1-x3 nonzero in violation of boot protocol:\n"
> "\tx1: %016llx\n\tx2: %016llx\n\tx3: %016llx\n"
> "This indicates a broken bootloader or old kernel\n",
> boot_args[1], boot_args[2], boot_args[3]);
>
OK, I have applied this change.
But I would like to note that we should probably only extend the boot
protocol in a way that would not trigger this on older kernels in the
first place.
I.e., assign a bit in the flags field in the header, which indicates
whether some boot protocol enhancement is supported by the kernel
being loaded, and only allow x1/x2/x3 to be non-zero if said
enhancement defines that.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-20 11:31 ` Ard Biesheuvel
@ 2015-03-20 11:41 ` Mark Rutland
2015-03-20 11:45 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Mark Rutland @ 2015-03-20 11:41 UTC (permalink / raw)
To: linux-arm-kernel
> >> + if (boot_args[1] || boot_args[2] || boot_args[3]) {
> >> + pr_err("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n",
> >> + boot_args[1], boot_args[2], boot_args[3]);
> >> + pr_err("WARNING: your bootloader may fail to load newer kernels\n");
> >
> > If we ever decide to use x1-x3 for something, and try to boot an older
> > kernel, that warning is going to be a bit misleading. That could matter
> > for VMs where we're going to see old kernel images for a long time.
> >
> > I would like the warning to mention that could be the case.
> >
> > It would also be nice if the message were consistently spaced regardless
> > of the values of x1-x3, so we should zero-pad them (and as that takes a
> > resonable amount of space, let's give them a line each).
> >
> > So could we change the warning to be something like:
> >
> > pr_err("WARNING: x1-x3 nonzero in violation of boot protocol:\n"
> > "\tx1: %016llx\n\tx2: %016llx\n\tx3: %016llx\n"
> > "This indicates a broken bootloader or old kernel\n",
> > boot_args[1], boot_args[2], boot_args[3]);
> >
>
> OK, I have applied this change.
>
> But I would like to note that we should probably only extend the boot
> protocol in a way that would not trigger this on older kernels in the
> first place.
> I.e., assign a bit in the flags field in the header, which indicates
> whether some boot protocol enhancement is supported by the kernel
> being loaded, and only allow x1/x2/x3 to be non-zero if said
> enhancement defines that.
Good point.
Given that, if you want to restore your original last line, that would
be fine with me (and my Ack still applies).
Mark.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-20 11:41 ` Mark Rutland
@ 2015-03-20 11:45 ` Ard Biesheuvel
2015-03-20 12:25 ` Will Deacon
0 siblings, 1 reply; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-20 11:45 UTC (permalink / raw)
To: linux-arm-kernel
On 20 March 2015 at 12:41, Mark Rutland <mark.rutland@arm.com> wrote:
>> >> + if (boot_args[1] || boot_args[2] || boot_args[3]) {
>> >> + pr_err("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n",
>> >> + boot_args[1], boot_args[2], boot_args[3]);
>> >> + pr_err("WARNING: your bootloader may fail to load newer kernels\n");
>> >
>> > If we ever decide to use x1-x3 for something, and try to boot an older
>> > kernel, that warning is going to be a bit misleading. That could matter
>> > for VMs where we're going to see old kernel images for a long time.
>> >
>> > I would like the warning to mention that could be the case.
>> >
>> > It would also be nice if the message were consistently spaced regardless
>> > of the values of x1-x3, so we should zero-pad them (and as that takes a
>> > resonable amount of space, let's give them a line each).
>> >
>> > So could we change the warning to be something like:
>> >
>> > pr_err("WARNING: x1-x3 nonzero in violation of boot protocol:\n"
>> > "\tx1: %016llx\n\tx2: %016llx\n\tx3: %016llx\n"
>> > "This indicates a broken bootloader or old kernel\n",
>> > boot_args[1], boot_args[2], boot_args[3]);
>> >
>>
>> OK, I have applied this change.
>>
>> But I would like to note that we should probably only extend the boot
>> protocol in a way that would not trigger this on older kernels in the
>> first place.
>> I.e., assign a bit in the flags field in the header, which indicates
>> whether some boot protocol enhancement is supported by the kernel
>> being loaded, and only allow x1/x2/x3 to be non-zero if said
>> enhancement defines that.
>
> Good point.
>
> Given that, if you want to restore your original last line, that would
> be fine with me (and my Ack still applies).
>
I think it's fine to leave it as is
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-20 11:45 ` Ard Biesheuvel
@ 2015-03-20 12:25 ` Will Deacon
2015-03-20 12:50 ` Ard Biesheuvel
0 siblings, 1 reply; 37+ messages in thread
From: Will Deacon @ 2015-03-20 12:25 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Mar 20, 2015 at 11:45:17AM +0000, Ard Biesheuvel wrote:
> On 20 March 2015 at 12:41, Mark Rutland <mark.rutland@arm.com> wrote:
> >> >> + if (boot_args[1] || boot_args[2] || boot_args[3]) {
> >> >> + pr_err("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n",
> >> >> + boot_args[1], boot_args[2], boot_args[3]);
> >> >> + pr_err("WARNING: your bootloader may fail to load newer kernels\n");
> >> >
> >> > If we ever decide to use x1-x3 for something, and try to boot an older
> >> > kernel, that warning is going to be a bit misleading. That could matter
> >> > for VMs where we're going to see old kernel images for a long time.
> >> >
> >> > I would like the warning to mention that could be the case.
> >> >
> >> > It would also be nice if the message were consistently spaced regardless
> >> > of the values of x1-x3, so we should zero-pad them (and as that takes a
> >> > resonable amount of space, let's give them a line each).
> >> >
> >> > So could we change the warning to be something like:
> >> >
> >> > pr_err("WARNING: x1-x3 nonzero in violation of boot protocol:\n"
> >> > "\tx1: %016llx\n\tx2: %016llx\n\tx3: %016llx\n"
> >> > "This indicates a broken bootloader or old kernel\n",
> >> > boot_args[1], boot_args[2], boot_args[3]);
> >> >
> >>
> >> OK, I have applied this change.
> >>
> >> But I would like to note that we should probably only extend the boot
> >> protocol in a way that would not trigger this on older kernels in the
> >> first place.
> >> I.e., assign a bit in the flags field in the header, which indicates
> >> whether some boot protocol enhancement is supported by the kernel
> >> being loaded, and only allow x1/x2/x3 to be non-zero if said
> >> enhancement defines that.
> >
> > Good point.
> >
> > Given that, if you want to restore your original last line, that would
> > be fine with me (and my Ack still applies).
> >
>
> I think it's fine to leave it as is
Yup, and this is sitting pretty on the arm64 devel branch.
Ard: I also pushed a kvm-bounce-page branch for you. Next step would be to
merge everything into for-next/core and put your VA changes on top of that.
I'd appreciate a sanity check of the current branch first, though!
Cheers,
Will
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol
2015-03-20 12:25 ` Will Deacon
@ 2015-03-20 12:50 ` Ard Biesheuvel
0 siblings, 0 replies; 37+ messages in thread
From: Ard Biesheuvel @ 2015-03-20 12:50 UTC (permalink / raw)
To: linux-arm-kernel
On 20 March 2015 at 13:25, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Mar 20, 2015 at 11:45:17AM +0000, Ard Biesheuvel wrote:
>> On 20 March 2015 at 12:41, Mark Rutland <mark.rutland@arm.com> wrote:
>> >> >> + if (boot_args[1] || boot_args[2] || boot_args[3]) {
>> >> >> + pr_err("WARNING: boot protocol violation detected (x1 == %llx, x2 == %llx, x3 == %llx)\n",
>> >> >> + boot_args[1], boot_args[2], boot_args[3]);
>> >> >> + pr_err("WARNING: your bootloader may fail to load newer kernels\n");
>> >> >
>> >> > If we ever decide to use x1-x3 for something, and try to boot an older
>> >> > kernel, that warning is going to be a bit misleading. That could matter
>> >> > for VMs where we're going to see old kernel images for a long time.
>> >> >
>> >> > I would like the warning to mention that could be the case.
>> >> >
>> >> > It would also be nice if the message were consistently spaced regardless
>> >> > of the values of x1-x3, so we should zero-pad them (and as that takes a
>> >> > resonable amount of space, let's give them a line each).
>> >> >
>> >> > So could we change the warning to be something like:
>> >> >
>> >> > pr_err("WARNING: x1-x3 nonzero in violation of boot protocol:\n"
>> >> > "\tx1: %016llx\n\tx2: %016llx\n\tx3: %016llx\n"
>> >> > "This indicates a broken bootloader or old kernel\n",
>> >> > boot_args[1], boot_args[2], boot_args[3]);
>> >> >
>> >>
>> >> OK, I have applied this change.
>> >>
>> >> But I would like to note that we should probably only extend the boot
>> >> protocol in a way that would not trigger this on older kernels in the
>> >> first place.
>> >> I.e., assign a bit in the flags field in the header, which indicates
>> >> whether some boot protocol enhancement is supported by the kernel
>> >> being loaded, and only allow x1/x2/x3 to be non-zero if said
>> >> enhancement defines that.
>> >
>> > Good point.
>> >
>> > Given that, if you want to restore your original last line, that would
>> > be fine with me (and my Ack still applies).
>> >
>>
>> I think it's fine to leave it as is
>
> Yup, and this is sitting pretty on the arm64 devel branch.
>
> Ard: I also pushed a kvm-bounce-page branch for you. Next step would be to
> merge everything into for-next/core and put your VA changes on top of that.
>
> I'd appreciate a sanity check of the current branch first, though!
>
OK, I pulled devel and put the KVM and VA branches on top, and
everything builds and runs fine afaict.
I also checked that Arnd's dotconfig from hell does not have the false
negative on the HYP text size assert.
"""
0x00000001 ASSERT ((((__hyp_idmap_text_start & 0xfff) +
__hyp_idmap_size) <= 0x1000), , HYP init code too big or misaligned)
"""
(Note that this .config does not even produce a bootable zImage.)
You will get a conflict in head.S when you merge the core VA range patch.
Nothing major but I'm happy to resend the patch.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 1/8] arm64: Get rid of struct cpu_table
2015-03-18 14:55 ` [PATCH v5 1/8] arm64: Get rid of struct cpu_table Ard Biesheuvel
2015-03-18 16:11 ` Mark Rutland
@ 2015-03-23 17:11 ` Suzuki K. Poulose
2015-03-23 17:38 ` Will Deacon
1 sibling, 1 reply; 37+ messages in thread
From: Suzuki K. Poulose @ 2015-03-23 17:11 UTC (permalink / raw)
To: linux-arm-kernel
On 18/03/15 14:55, Ard Biesheuvel wrote:
> From: Marc Zyngier <marc.zyngier@arm.com>
>
> struct cpu_table is an artifact left from the (very) early days of
> the arm64 port, and its only real use is to allow the most beautiful
> "AArch64 Processor" string to be displayed at boot time.
>
> Really? Yes, really.
>
> Let's get rid of it. In order to avoid another BogoMips-gate, the
> aforementioned string is preserved.
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> arch/arm64/include/asm/cputable.h | 30 ----------------
> arch/arm64/kernel/Makefile | 2 +-
> arch/arm64/kernel/asm-offsets.c | 4 ---
> arch/arm64/kernel/cputable.c | 33 -----------------
> arch/arm64/kernel/head.S | 76 +++------------------------------------
> arch/arm64/kernel/setup.c | 16 ++-------
> 6 files changed, 8 insertions(+), 153 deletions(-)
> delete mode 100644 arch/arm64/include/asm/cputable.h
> delete mode 100644 arch/arm64/kernel/cputable.c
...
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -50,7 +50,6 @@
> #include <asm/cpu.h>
> #include <asm/cputype.h>
> #include <asm/elf.h>
> -#include <asm/cputable.h>
> #include <asm/cpufeature.h>
> #include <asm/cpu_ops.h>
> #include <asm/sections.h>
> @@ -83,7 +82,6 @@ unsigned int compat_elf_hwcap2 __read_mostly;
>
> DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
>
> -static const char *cpu_name;
> phys_addr_t __fdt_pointer __initdata;
>
> /*
> @@ -209,22 +207,12 @@ static void __init smp_build_mpidr_hash(void)
>
> static void __init setup_processor(void)
> {
> - struct cpu_info *cpu_info;
> u64 features, block;
> u32 cwg;
> int cls;
>
> - cpu_info = lookup_processor_type(read_cpuid_id());
> - if (!cpu_info) {
> - printk("CPU configuration botched (ID %08x), unable to continue.\n",
> - read_cpuid_id());
> - while (1);
> - }
> -
> - cpu_name = cpu_info->cpu_name;
> -
> - printk("CPU: %s [%08x] revision %d\n",
> - cpu_name, read_cpuid_id(), read_cpuid_id() & 15);
> + printk("CPU: AArch64 Processor [%08x] revision %d\n",
> + read_cpuid_id(), read_cpuid_id() & 15);
>
While we are at it, does it make sense to change
s/CPU/Boot CPU/
to make it clear on a big.Little system ?
Suzuki
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 1/8] arm64: Get rid of struct cpu_table
2015-03-23 17:11 ` Suzuki K. Poulose
@ 2015-03-23 17:38 ` Will Deacon
2015-03-23 17:41 ` Suzuki K. Poulose
0 siblings, 1 reply; 37+ messages in thread
From: Will Deacon @ 2015-03-23 17:38 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 23, 2015 at 05:11:02PM +0000, Suzuki K. Poulose wrote:
> On 18/03/15 14:55, Ard Biesheuvel wrote:
> > @@ -209,22 +207,12 @@ static void __init smp_build_mpidr_hash(void)
> >
> > static void __init setup_processor(void)
> > {
> > - struct cpu_info *cpu_info;
> > u64 features, block;
> > u32 cwg;
> > int cls;
> >
> > - cpu_info = lookup_processor_type(read_cpuid_id());
> > - if (!cpu_info) {
> > - printk("CPU configuration botched (ID %08x), unable to continue.\n",
> > - read_cpuid_id());
> > - while (1);
> > - }
> > -
> > - cpu_name = cpu_info->cpu_name;
> > -
> > - printk("CPU: %s [%08x] revision %d\n",
> > - cpu_name, read_cpuid_id(), read_cpuid_id() & 15);
> > + printk("CPU: AArch64 Processor [%08x] revision %d\n",
> > + read_cpuid_id(), read_cpuid_id() & 15);
> >
>
> While we are at it, does it make sense to change
>
> s/CPU/Boot CPU/
>
> to make it clear on a big.Little system ?
This is already queued, so I don't think it's worth the effort now.
Will
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v5 1/8] arm64: Get rid of struct cpu_table
2015-03-23 17:38 ` Will Deacon
@ 2015-03-23 17:41 ` Suzuki K. Poulose
0 siblings, 0 replies; 37+ messages in thread
From: Suzuki K. Poulose @ 2015-03-23 17:41 UTC (permalink / raw)
To: linux-arm-kernel
On 23/03/15 17:38, Will Deacon wrote:
> On Mon, Mar 23, 2015 at 05:11:02PM +0000, Suzuki K. Poulose wrote:
>> On 18/03/15 14:55, Ard Biesheuvel wrote:
>>> @@ -209,22 +207,12 @@ static void __init smp_build_mpidr_hash(void)
>>>
>>> static void __init setup_processor(void)
>>> {
>>> - struct cpu_info *cpu_info;
>>> u64 features, block;
>>> u32 cwg;
>>> int cls;
>>>
>>> - cpu_info = lookup_processor_type(read_cpuid_id());
>>> - if (!cpu_info) {
>>> - printk("CPU configuration botched (ID %08x), unable to continue.\n",
>>> - read_cpuid_id());
>>> - while (1);
>>> - }
>>> -
>>> - cpu_name = cpu_info->cpu_name;
>>> -
>>> - printk("CPU: %s [%08x] revision %d\n",
>>> - cpu_name, read_cpuid_id(), read_cpuid_id() & 15);
>>> + printk("CPU: AArch64 Processor [%08x] revision %d\n",
>>> + read_cpuid_id(), read_cpuid_id() & 15);
>>>
>>
>> While we are at it, does it make sense to change
>>
>> s/CPU/Boot CPU/
>>
>> to make it clear on a big.Little system ?
>
> This is already queued, so I don't think it's worth the effort now.
ok, not a major one. I will fix it in one of my series
Suzuki
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2015-03-23 17:41 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-18 14:55 [PATCH v5 0/8] arm64: head.S cleanup Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 1/8] arm64: Get rid of struct cpu_table Ard Biesheuvel
2015-03-18 16:11 ` Mark Rutland
2015-03-23 17:11 ` Suzuki K. Poulose
2015-03-23 17:38 ` Will Deacon
2015-03-23 17:41 ` Suzuki K. Poulose
2015-03-18 14:55 ` [PATCH v5 2/8] arm64: add macros for common adrp usages Ard Biesheuvel
2015-03-18 17:54 ` Mark Rutland
2015-03-18 17:56 ` Ard Biesheuvel
2015-03-18 18:05 ` Mark Rutland
2015-03-18 18:06 ` Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 3/8] arm64: remove processor_id Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 4/8] arm64: remove __switch_data object from head.S Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 5/8] arm64: use PC-relative reference for secondary_holding_pen_release Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 6/8] arm64: merge __enable_mmu and __turn_mmu_on Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 7/8] arm64: remove __calc_phys_offset Ard Biesheuvel
2015-03-18 14:55 ` [PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol Ard Biesheuvel
2015-03-18 18:13 ` Mark Rutland
2015-03-18 18:16 ` Ard Biesheuvel
2015-03-18 18:46 ` Ard Biesheuvel
2015-03-18 18:57 ` Mark Rutland
2015-03-18 19:55 ` Ard Biesheuvel
2015-03-18 20:24 ` Mark Rutland
2015-03-19 7:30 ` Ard Biesheuvel
2015-03-19 10:35 ` Mark Rutland
2015-03-19 10:38 ` Ard Biesheuvel
2015-03-19 10:41 ` Mark Rutland
2015-03-19 11:00 ` [PATCH v3] " Ard Biesheuvel
2015-03-19 13:36 ` Mark Rutland
2015-03-20 11:31 ` Ard Biesheuvel
2015-03-20 11:41 ` Mark Rutland
2015-03-20 11:45 ` Ard Biesheuvel
2015-03-20 12:25 ` Will Deacon
2015-03-20 12:50 ` Ard Biesheuvel
2015-03-18 22:26 ` [PATCH v5 8/8] " Peter Maydell
2015-03-18 18:23 ` [PATCH v5 0/8] arm64: head.S cleanup Mark Rutland
2015-03-18 18:28 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).