[PATCH v5 0/8] arm64: head.S cleanup

[PATCH v5 0/8] arm64: head.S cleanup

[Ard Biesheuvel]

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

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

[PATCH v5 1/8] arm64: Get rid of struct cpu_table

[Ard Biesheuvel]

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

[PATCH v5 2/8] arm64: add macros for common adrp usages

[Ard Biesheuvel]

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

[PATCH v5 3/8] arm64: remove processor_id

[Ard Biesheuvel]

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

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

[PATCH v5 4/8] arm64: remove __switch_data object from head.S

[Ard Biesheuvel]

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

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

[PATCH v5 5/8] arm64: use PC-relative reference for secondary_holding_pen_release

[Ard Biesheuvel]

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

[PATCH v5 6/8] arm64: merge __enable_mmu and __turn_mmu_on

[Ard Biesheuvel]

Enabling of the MMU is split into two functions, with an align and
a branch in the middle. On arm64, the entire kernel Image is ID mapped
so this is really not necessary, and we can just merge it into a
single function.

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

[PATCH v5 7/8] arm64: remove __calc_phys_offset

[Ard Biesheuvel]

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

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

According to the arm64 boot protocol, registers x1 to x3 should be
zero upon kernel entry, and non-zero values are reserved for future
use. This future use is going to be problematic if we never enforce
the current rules, so start enforcing them now, by emitting a warning
if non-zero values are detected.

Signed-off-by: Ard Biesheuvel <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

[PATCH v5 1/8] arm64: Get rid of struct cpu_table

[Mark Rutland]

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
> 
> 

[PATCH v5 2/8] arm64: add macros for common adrp usages

[Mark Rutland]

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
> 
> 

[PATCH v5 2/8] arm64: add macros for common adrp usages

[Ard Biesheuvel]

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
>>
>>

[PATCH v5 2/8] arm64: add macros for common adrp usages

[Mark Rutland]

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.

[PATCH v5 2/8] arm64: add macros for common adrp usages

[Ard Biesheuvel]

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

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Mark Rutland]

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
> 
> 

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

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
>>
>>

[PATCH v5 0/8] arm64: head.S cleanup

[Mark Rutland]

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
> 
> 

[PATCH v5 0/8] arm64: head.S cleanup

[Ard Biesheuvel]

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
>>
>>

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

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
>>>
>>>

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Mark Rutland]

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.

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

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

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Mark Rutland]

> >> >>>  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.

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Peter Maydell]

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

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

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?

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Mark Rutland]

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.

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

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.

[PATCH v5 8/8] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Mark Rutland]

> >> 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.

[PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

According to the arm64 boot protocol, registers x1 to x3 should be
zero upon kernel entry, and non-zero values are reserved for future
use. This future use is going to be problematic if we never enforce
the current rules, so start enforcing them now, by emitting a warning
if non-zero values are detected.

Signed-off-by: Ard Biesheuvel <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

[PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Mark Rutland]

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.

[PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

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.

[PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Mark Rutland]

> >> +     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.

[PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

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

[PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Will Deacon]

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

[PATCH v3] arm64: enforce x1|x2|x3 == 0 upon kernel entry as per boot protocol

[Ard Biesheuvel]

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.

[PATCH v5 1/8] arm64: Get rid of struct cpu_table

[Suzuki K. Poulose]

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

[PATCH v5 1/8] arm64: Get rid of struct cpu_table

[Will Deacon]

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

[PATCH v5 1/8] arm64: Get rid of struct cpu_table

[Suzuki K. Poulose]

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