linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup
@ 2021-07-29 15:20 Mark Rutland
  2021-07-29 15:20 ` [boot-wrapper PATCH 01/12] Ensure `kernel_address` is aligned Mark Rutland
                   ` (11 more replies)
  0 siblings, 12 replies; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

The boot-wrapper has had periodic updates over the last few years, and with
each addition becomes increasingly in need of some larger scale cleanup to keep
it clear and maintainable.

While looking over Jaxson Han's ARMv8-R AArch64 series [1] I realised there are
a number of existing issues and design warts that we need to clean up before it
becomes harder to do so.

These patches address some existing issues with the boot-wrapper, and
restructure the codebase to make it easier to navigate. I intend to merge these
in the next few days if there are no objections, and will follow up with some
further rework which should make it easier to add ARMv8-R AArch64 support atop.

Thanks,
Mark.

[1] https://lore.kernel.org/r/20210525062509.201464-1-jaxson.han@arm.com

Mark Rutland (12):
  Ensure `kernel_address` is aligned
  Output text separately from data
  Remove cache maintenance
  Remove `flag_no_el3`
  Move PSCI triage to C
  Move scripts to a `scripts` directory
  aarch64: respect text offset
  Consistently use logical CPU IDs
  Cleanup `.globl` usage
  aarch32: rename `_spin_dead` -> `err_invalid_id`
  Rename `spin.h` -> `boot.h`
  Move common source files to `common` directory

 Makefile.am                            | 53 ++++++++++++---------
 arch/aarch32/boot.S                    | 22 +++------
 arch/aarch32/psci.S                    | 27 ++---------
 arch/aarch32/stack.S                   | 11 ++---
 arch/aarch32/utils.S                   |  4 +-
 arch/aarch64/boot.S                    | 21 ++------
 arch/aarch64/psci.S                    | 30 ++----------
 arch/aarch64/spin.S                    | 10 ++--
 arch/aarch64/stack.S                   | 11 ++---
 arch/aarch64/utils.S                   |  9 ++--
 cache.c                                | 58 -----------------------
 bakery_lock.c => common/bakery_lock.c  |  0
 boot_common.c => common/boot.c         |  7 +--
 gic-v3.c => common/gic-v3.c            |  4 +-
 gic.c => common/gic.c                  |  3 +-
 lib.c => common/lib.c                  |  0
 platform.c => common/platform.c        |  0
 psci.c => common/psci.c                | 28 ++++++++---
 include/{spin.h => boot.h}             |  6 +--
 include/cpu.h                          |  2 +
 include/linkage.h                      |  6 ++-
 model.lds.S                            |  3 +-
 scripts/AA64Image.pm                   | 87 ++++++++++++++++++++++++++++++++++
 FDT.pm => scripts/FDT.pm               |  0
 scripts/aa64-load-offset.pl            | 28 +++++++++++
 addpsci.pl => scripts/addpsci.pl       |  0
 findbase.pl => scripts/findbase.pl     |  0
 findcpuids.pl => scripts/findcpuids.pl |  0
 findmem.pl => scripts/findmem.pl       |  0
 29 files changed, 218 insertions(+), 212 deletions(-)
 delete mode 100644 cache.c
 rename bakery_lock.c => common/bakery_lock.c (100%)
 rename boot_common.c => common/boot.c (93%)
 rename gic-v3.c => common/gic-v3.c (98%)
 rename gic.c => common/gic.c (96%)
 rename lib.c => common/lib.c (100%)
 rename platform.c => common/platform.c (100%)
 rename psci.c => common/psci.c (69%)
 rename include/{spin.h => boot.h} (88%)
 create mode 100755 scripts/AA64Image.pm
 rename FDT.pm => scripts/FDT.pm (100%)
 create mode 100755 scripts/aa64-load-offset.pl
 rename addpsci.pl => scripts/addpsci.pl (100%)
 rename findbase.pl => scripts/findbase.pl (100%)
 rename findcpuids.pl => scripts/findcpuids.pl (100%)
 rename findmem.pl => scripts/findmem.pl (100%)

-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 01/12] Ensure `kernel_address` is aligned
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 15:11   ` Andre Przywara
  2021-07-29 15:20 ` [boot-wrapper PATCH 02/12] Output text separately from data Mark Rutland
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

We accidentally placed the `.align` directive after the `kernel_address`
label, meaning that the label itself isn't necessarily aligned. Place
the `.align` directive first to ensure this.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch64/spin.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/aarch64/spin.S b/arch/aarch64/spin.S
index 72603cf..ca05937 100644
--- a/arch/aarch64/spin.S
+++ b/arch/aarch64/spin.S
@@ -30,8 +30,8 @@ start_no_el3:
 	mov	x2, #0
 	bl	first_spin
 
-kernel_address:
 	.align 3
+kernel_address:
 	.long 0
 
 	.ltorg
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 02/12] Output text separately from data
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
  2021-07-29 15:20 ` [boot-wrapper PATCH 01/12] Ensure `kernel_address` is aligned Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-29 15:20 ` [boot-wrapper PATCH 03/12] Remove cache maintenance Mark Rutland
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

Assembly files generally assume that .text is word-aligned, and don't
explciitly align the .text section. However, if we mix .text with data
sections at link time, we can output .text sections at less than word
alignment, resulting in boot-time hangs that are painful to debug.

Output all .text sections before .data sections to minimize this risk.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 model.lds.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/model.lds.S b/model.lds.S
index 370ff56..d4e7e13 100644
--- a/model.lds.S
+++ b/model.lds.S
@@ -65,7 +65,8 @@ SECTIONS
 
 	.boot PHYS_OFFSET: {
 		*(.init)
-		*(.text* .data* .rodata* .bss* COMMON)
+		*(.text*)
+		*(.data* .rodata* .bss* COMMON)
 		*(.vectors)
 		*(.stack)
 		PROVIDE(etext = .);
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 03/12] Remove cache maintenance
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
  2021-07-29 15:20 ` [boot-wrapper PATCH 01/12] Ensure `kernel_address` is aligned Mark Rutland
  2021-07-29 15:20 ` [boot-wrapper PATCH 02/12] Output text separately from data Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 15:12   ` Andre Przywara
  2021-07-29 15:20 ` [boot-wrapper PATCH 04/12] Remove `flag_no_el3` Mark Rutland
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

For models, we assume that out-of-reset caches ar invalid and no cache
maintenance is required.

We added cache maintenance to the boot-wrapper in commit:

  28ec269a22c8dc14 ("Add code to clean and invalidate caches")

... because the boot-wrapper would teansiently use cacheable mappings,
and could allocate into caches. As we were using Set/Way operations, we
were on somewhat shaky ground (e.g. due to system-level caches, or
dirty line migration). Further, we never took FEAT_CCIDX into account,
and so would not necessarily invalidate all potential levels of cache

However, since commit:

  0bb7b2545582accf ("Remove MMU identity map setup")

... we no longer enable the MMU within the boot-wrapper, and so no
longer have any reason to perform cache maintenance.

This patch removes the redundant and incomplete cache maintenance.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 Makefile.am   |  2 +-
 boot_common.c |  3 ---
 cache.c       | 58 ----------------------------------------------------------
 3 files changed, 1 insertion(+), 62 deletions(-)
 delete mode 100644 cache.c

diff --git a/Makefile.am b/Makefile.am
index ef6b793..8334049 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -123,7 +123,7 @@ CFLAGS		+= -Wall -fomit-frame-pointer
 CFLAGS		+= -ffunction-sections -fdata-sections
 LDFLAGS		+= --gc-sections
 
-OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) cache.o lib.o
+OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) lib.o
 OFILES		+= $(addprefix $(ARCH_SRC),boot.o stack.o $(BOOTMETHOD) utils.o)
 
 # Don't lookup all prerequisites in $(top_srcdir), only the source files. When
diff --git a/boot_common.c b/boot_common.c
index e7b8e1d..d48b7e1 100644
--- a/boot_common.c
+++ b/boot_common.c
@@ -13,7 +13,6 @@ extern unsigned long entrypoint;
 extern unsigned long dtb;
 
 void init_platform(void);
-void flush_caches(void);
 
 void __noreturn jump_kernel(unsigned long address,
 			    unsigned long a0,
@@ -62,8 +61,6 @@ void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry)
 void __noreturn first_spin(unsigned int cpu, unsigned long *mbox,
 			   unsigned long invalid)
 {
-	flush_caches();
-
 	if (cpu == 0) {
 		init_platform();
 
diff --git a/cache.c b/cache.c
deleted file mode 100644
index 9d71248..0000000
--- a/cache.c
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
- * cache.c - simple cache clean+invalidate code
- *
- * Copyright (C) 2015 ARM Limited. All rights reserved.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE.txt file.
- */
-
-#include <cpu.h>
-
-void flush_caches(void)
-{
-	unsigned int level;
-	uint32_t clidr = read_clidr();
-	unsigned int max_level = (clidr >> 24) & 0x7;
-
-	uint32_t ccsidr;
-
-	if (max_level == 0)
-		return;
-
-	for (level = 0; level < max_level; level++) {
-		uint32_t cache_type = (clidr >> (level * 3)) & 0x7;
-		unsigned int line_size, num_ways, num_sets, way_shift;
-		unsigned int way, set;
-
-		if (cache_type == 1)
-			/* No dcache at this level */
-			continue;
-
-		write_csselr(level << 1);
-		isb();
-		ccsidr = read_ccsidr();
-
-		line_size = (ccsidr & 0x7) + 4; /* log2 line size */
-		num_ways = ((ccsidr >> 3) & 0x3ff) + 1;
-		num_sets = ((ccsidr >> 13) & 0x7fff) + 1;
-
-		way_shift = clz(num_ways - 1);
-		for (way = 0; way < num_ways; way++) {
-			for (set = 0; set < num_sets; set++) {
-				uint32_t command = level << 1;
-				command |= way << way_shift;
-				command |= set << line_size;
-
-				dccisw(command);
-				dsb(sy);
-			}
-		}
-
-		dsb(sy);
-	}
-	dsb(sy);
-	iciallu();
-	dsb(sy);
-	isb();
-}
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 04/12] Remove `flag_no_el3`
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
                   ` (2 preceding siblings ...)
  2021-07-29 15:20 ` [boot-wrapper PATCH 03/12] Remove cache maintenance Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 15:13   ` Andre Przywara
  2021-07-29 15:20 ` [boot-wrapper PATCH 05/12] Move PSCI triage to C Mark Rutland
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

We set `flag_no_el3` when not booted at EL3 / monitor mode, and
subsequently we use this to determine whether we need to drop exception
level before entering Linux. As this can be derived from CurrentEL or
CPSR, the flag itself is redundant, and we can defer the check until
we're about to enter Linux.

In future this will allow more logic to be converted into C, where it
will be easier to handle architectural variants.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch32/boot.S | 14 +++-----------
 arch/aarch64/boot.S | 13 ++-----------
 2 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index 2a85ad5..0bd1ca2 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -31,9 +31,6 @@ ENTRY(_start)
 	cmp	r0, #PSR_HYP
 	bne	_switch_monitor
 
-	mov	r0, #1
-	ldr	r1, =flag_no_el3
-	str	r0, [r1]
 	b	start_no_el3
 
 _switch_monitor:
@@ -89,9 +86,9 @@ ENTRY(jump_kernel)
 	ldr	lr, [r5], #4
 	ldm	r5, {r0 - r2}
 
-	ldr	r4, =flag_no_el3
-	ldr	r4, [r4]
-	cmp	r4, #1
+	mrs	r4, cpsr
+	and	r4, #PSR_MODE_MASK
+	cmp	r4, #PSR_MON
 	bxeq	lr				@ no EL3
 
 	ldr	r4, =SPSR_KERNEL
@@ -113,8 +110,3 @@ boot_vectors:
 	b	.
 	b	.
 	b	.
-
-	.section .data
-	.align 2
-flag_no_el3:
-	.long 0
diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index 37759ce..fae0188 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -28,10 +28,6 @@ _start:
 	cmp	x0, #CURRENTEL_EL3
 	b.eq	1f
 
-	mov	w0, #1
-	ldr	x1, =flag_no_el3
-	str	w0, [x1]
-
 	b	start_no_el3
 
 1:	mov	x0, #0x30			// RES1
@@ -140,8 +136,8 @@ jump_kernel:
 	bl	find_logical_id
 	bl	setup_stack		// Reset stack pointer
 
-	ldr	w0, flag_no_el3
-	cmp	w0, #0			// Prepare Z flag
+	mrs	x0, CurrentEl
+	cmp	w0, #CURRENTEL_EL3	// Prepare Z flag
 
 	mov	x0, x20
 	mov	x1, x21
@@ -164,8 +160,3 @@ jump_kernel:
 	eret
 
 	.ltorg
-
-	.data
-	.align 3
-flag_no_el3:
-	.long 0
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 05/12] Move PSCI triage to C
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
                   ` (3 preceding siblings ...)
  2021-07-29 15:20 ` [boot-wrapper PATCH 04/12] Remove `flag_no_el3` Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-29 15:20 ` [boot-wrapper PATCH 06/12] Move scripts to a `scripts` directory Mark Rutland
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

There's no reason we need to test the PSCI function IDs in assembly;
move this to C so that it can be shared across AArch64 and AArch32. At
the same time, limit the PSCI_CPU_ON FIDs to match the register width of
the kernel.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch32/psci.S | 23 +----------------------
 arch/aarch64/psci.S | 21 +--------------------
 psci.c              | 21 +++++++++++++++++++--
 3 files changed, 21 insertions(+), 44 deletions(-)

diff --git a/arch/aarch32/psci.S b/arch/aarch32/psci.S
index 78dca96..0b7663e 100644
--- a/arch/aarch32/psci.S
+++ b/arch/aarch32/psci.S
@@ -33,32 +33,11 @@ handle_smc:
 	@ Follow the SMC32 calling convention: preserve r4 - r14
 	push	{r4 - r12, lr}
 
-	ldr	r4, =PSCI_CPU_ON_32
-	cmp	r4, r0
-	ldr	r4, =psci_cpu_on
-	beq	do_psci_call
-
-	ldr	r4, =PSCI_CPU_OFF
-	cmp	r4, r0
-	ldr	r4, =psci_cpu_off
-	beq	do_psci_call
-
-	adr	r4, psci_invalid
-
-do_psci_call:
-	mov	r0, r1
-	mov	r1, r2
-	mov	r2, r3
-
-	blx	r4
+	blx	psci_call
 
 	pop	{r4 - r12, lr}
 	movs	pc, lr
 
-psci_invalid:
-	mov	r0, #PSCI_RET_NOT_SUPPORTED
-	bx	lr
-
 ENTRY(start_el3)
 	ldr	r0, =smc_vectors
 	blx	setup_vector
diff --git a/arch/aarch64/psci.S b/arch/aarch64/psci.S
index 01ebe7d..6dbca11 100644
--- a/arch/aarch64/psci.S
+++ b/arch/aarch64/psci.S
@@ -51,13 +51,6 @@ vector:
 err_exception:
 	b err_exception
 
-	.macro branch_if val, addr
-	ldr	x7, =\val
-	cmp	x0, x7
-	adr	x7, \addr
-	b.eq	do_call
-	.endm
-
 smc_entry32:
 	/* Clear upper bits */
 	mov	w0, w0
@@ -74,22 +67,10 @@ smc_entry64:
 	// Keep sp aligned to 16 bytes
 	stp	x30, xzr, [sp, #-16]!
 
-	/* If function ID matches, do_call with procedure address in x7 */
-	branch_if PSCI_CPU_ON_32,	psci_cpu_on
-	branch_if PSCI_CPU_ON_64,	psci_cpu_on
-	branch_if PSCI_CPU_OFF,		psci_cpu_off
+	bl	psci_call
 
-	/* Otherwise, return error in x0/w0 */
-	mov	x0, PSCI_RET_NOT_SUPPORTED
 	b	smc_exit
 
-do_call:
-	mov	x0, x1
-	mov	x1, x2
-	mov	x2, x3
-
-	blr	x7
-
 smc_exit:
 	ldp	x30, xzr, [sp], #16
 	ldp	x28, x29, [sp], #16
diff --git a/psci.c b/psci.c
index fad6f5d..e5d54b7 100644
--- a/psci.c
+++ b/psci.c
@@ -31,7 +31,7 @@ static int psci_store_address(unsigned int cpu, unsigned long address)
 	return PSCI_RET_SUCCESS;
 }
 
-int psci_cpu_on(unsigned long target_mpidr, unsigned long address)
+static int psci_cpu_on(unsigned long target_mpidr, unsigned long address)
 {
 	int ret;
 	unsigned int cpu = find_logical_id(target_mpidr);
@@ -47,7 +47,7 @@ int psci_cpu_on(unsigned long target_mpidr, unsigned long address)
 	return ret;
 }
 
-int psci_cpu_off(void)
+static int psci_cpu_off(void)
 {
 	unsigned long mpidr = read_mpidr();
 	unsigned int cpu = find_logical_id(mpidr);
@@ -62,6 +62,23 @@ int psci_cpu_off(void)
 	unreachable();
 }
 
+long psci_call(unsigned long fid, unsigned long arg1, unsigned long arg2)
+{
+	switch (fid) {
+	case PSCI_CPU_OFF:
+		return psci_cpu_off();
+#ifdef KERNEL_32
+	case PSCI_CPU_ON_32:
+		return psci_cpu_on(arg1, arg2);
+#else
+	case PSCI_CPU_ON_64:
+		return psci_cpu_on(arg1, arg2);
+#endif
+	default:
+		return PSCI_RET_NOT_SUPPORTED;
+	}
+}
+
 void __noreturn psci_first_spin(unsigned int cpu)
 {
 	if (cpu == MPIDR_INVALID)
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 06/12] Move scripts to a `scripts` directory
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
                   ` (4 preceding siblings ...)
  2021-07-29 15:20 ` [boot-wrapper PATCH 05/12] Move PSCI triage to C Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 15:13   ` Andre Przywara
  2021-07-29 15:20 ` [boot-wrapper PATCH 07/12] aarch64: respect text offset Mark Rutland
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

The top-level directory is getting increasingly cluttered. For clarity
let's move the scripts into their own directory.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 Makefile.am                            | 20 +++++++++++---------
 FDT.pm => scripts/FDT.pm               |  0
 addpsci.pl => scripts/addpsci.pl       |  0
 findbase.pl => scripts/findbase.pl     |  0
 findcpuids.pl => scripts/findcpuids.pl |  0
 findmem.pl => scripts/findmem.pl       |  0
 6 files changed, 11 insertions(+), 9 deletions(-)
 rename FDT.pm => scripts/FDT.pm (100%)
 rename addpsci.pl => scripts/addpsci.pl (100%)
 rename findbase.pl => scripts/findbase.pl (100%)
 rename findcpuids.pl => scripts/findcpuids.pl (100%)
 rename findmem.pl => scripts/findmem.pl (100%)

diff --git a/Makefile.am b/Makefile.am
index 8334049..ad13dbc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -7,13 +7,15 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE.txt file.
 
+SCRIPT_DIR	:= $(top_srcdir)/scripts
+
 # VE
-PHYS_OFFSET	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findmem.pl $(KERNEL_DTB))
-UART_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 0 'arm,pl011')
-SYSREGS_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 0 'arm,vexpress-sysreg' 2> /dev/null)
+PHYS_OFFSET	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findmem.pl $(KERNEL_DTB))
+UART_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,pl011')
+SYSREGS_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,vexpress-sysreg' 2> /dev/null)
 CNTFRQ		:= 0x01800000	# 24Mhz
 
-CPU_IDS		:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findcpuids.pl $(KERNEL_DTB))
+CPU_IDS		:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findcpuids.pl $(KERNEL_DTB))
 NR_CPUS         := $(shell echo $(CPU_IDS) | tr ',' ' ' | wc -w)
 
 DEFINES		= -DCNTFRQ=$(CNTFRQ)
@@ -51,7 +53,7 @@ PSCI_NODE	:= psci {				\
 			cpu_on = <$(PSCI_CPU_ON)>;	\
 			cpu_off = <$(PSCI_CPU_OFF)>;	\
 		   };
-CPU_NODES	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/addpsci.pl $(KERNEL_DTB))
+CPU_NODES	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/addpsci.pl $(KERNEL_DTB))
 else
 BOOTMETHOD	:= spin.o
 PSCI_NODE	:=
@@ -59,14 +61,14 @@ CPU_NODES	:=
 endif
 
 if GICV3
-GIC_DIST_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 0 'arm,gic-v3')
-GIC_RDIST_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3')
+GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,gic-v3')
+GIC_RDIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3')
 DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
 DEFINES		+= -DGIC_RDIST_BASE=$(GIC_RDIST_BASE)
 GIC		:= gic-v3.o
 else
-GIC_DIST_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic')
-GIC_CPU_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic')
+GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic')
+GIC_CPU_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic')
 DEFINES		+= -DGIC_CPU_BASE=$(GIC_CPU_BASE)
 DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
 GIC		:= gic.o
diff --git a/FDT.pm b/scripts/FDT.pm
similarity index 100%
rename from FDT.pm
rename to scripts/FDT.pm
diff --git a/addpsci.pl b/scripts/addpsci.pl
similarity index 100%
rename from addpsci.pl
rename to scripts/addpsci.pl
diff --git a/findbase.pl b/scripts/findbase.pl
similarity index 100%
rename from findbase.pl
rename to scripts/findbase.pl
diff --git a/findcpuids.pl b/scripts/findcpuids.pl
similarity index 100%
rename from findcpuids.pl
rename to scripts/findcpuids.pl
diff --git a/findmem.pl b/scripts/findmem.pl
similarity index 100%
rename from findmem.pl
rename to scripts/findmem.pl
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 07/12] aarch64: respect text offset
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
                   ` (5 preceding siblings ...)
  2021-07-29 15:20 ` [boot-wrapper PATCH 06/12] Move scripts to a `scripts` directory Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 15:13   ` Andre Przywara
  2021-07-29 15:20 ` [boot-wrapper PATCH 08/12] Consistently use logical CPU IDs Mark Rutland
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

The boot-wrapper assumes that an AArch64 kernel's text offset is 0x80000
rather than reading the `text_offset` field from the Image header as the
documentation says it should.

Add a script to figure this out during the build process. As with FDT.pm
the parsing of the Image (and common logic associated with this) is
factored into a module that we may use in more scripts in future.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 Makefile.am                 |  4 +--
 scripts/AA64Image.pm        | 87 +++++++++++++++++++++++++++++++++++++++++++++
 scripts/aa64-load-offset.pl | 28 +++++++++++++++
 3 files changed, 117 insertions(+), 2 deletions(-)
 create mode 100755 scripts/AA64Image.pm
 create mode 100755 scripts/aa64-load-offset.pl

diff --git a/Makefile.am b/Makefile.am
index ad13dbc..5d34cc8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -76,12 +76,12 @@ endif
 
 if KERNEL_32
 MBOX_OFFSET	:= 0x7ff8
-KERNEL_OFFSET	:= 0x8000
 TEXT_LIMIT	:= 0x3000
+KERNEL_OFFSET	:= 0x8000
 else
 MBOX_OFFSET	:= 0xfff8
-KERNEL_OFFSET	:= 0x80000
 TEXT_LIMIT	:= 0x80000
+KERNEL_OFFSET	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/aa64-load-offset.pl $(KERNEL_IMAGE) $(TEXT_LIMIT))
 endif
 
 LD_SCRIPT	:= model.lds.S
diff --git a/scripts/AA64Image.pm b/scripts/AA64Image.pm
new file mode 100755
index 0000000..36b2547
--- /dev/null
+++ b/scripts/AA64Image.pm
@@ -0,0 +1,87 @@
+#!/usr/bin/perl -w
+# A simple Flattened Device Tree Blob (FDT/DTB) parser.
+#
+# Copyright (C) 2014 ARM Limited. All rights reserved.
+#
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE.txt file.
+
+use warnings;
+use strict;
+use integer;
+
+package AA64Image;
+
+# Header definitions from v5.13
+# See https://www.kernel.org/doc/html/v5.13/arm64/booting.html#call-the-kernel-image
+
+use constant {
+	HEADER_LEN => 64,
+	HEADER_MAGIC => 0x644d5241,
+};
+
+sub parse
+{
+	my $class = shift;
+	my $fh = shift;
+	my $self = bless {}, $class;
+
+	read($fh, my $raw_header, AA64Image::HEADER_LEN) == AA64Image::HEADER_LEN or goto failed;
+
+	(
+		$self->{code0},
+		$self->{code1},
+		$self->{text_offset},
+		$self->{image_size},
+		$self->{flags},
+		$self->{res2},
+		$self->{res3},
+		$self->{res4},
+		$self->{magic},
+		$self->{res5}
+	) = unpack("VVQ<Q<Q<Q<Q<Q<VV", $raw_header);
+
+	if ($self->{magic} != AA64Image::HEADER_MAGIC) {
+		warn "Image header magic not found";
+		goto failed;
+	}
+
+	return $self;
+
+failed:
+	warn "Unable to parse header";
+	return undef;
+}
+
+sub get_text_offset
+{
+	my $self = shift;
+
+	# Where image_size is 0, the load offset can be assumed to be 0x80000
+	# See https://www.kernel.org/doc/html/v5.13/arm64/booting.html#call-the-kernel-image
+	if ($self->{image_size} == 0) {
+		return 0x80000;
+	}
+
+	return $self->{text_offset};
+}
+
+sub get_load_offset
+{
+	my $self = shift;
+	my $min = shift;
+	my $offset = $self->get_text_offset();
+
+	if ($min < $offset) {
+		return $offset;
+	}
+
+	# The image must be placed text_offset bytes from a 2MB aligned base address
+	my $size_2m = 2 * 1024 * 1024;
+	$min += $size_2m - 1;
+	$min &= ~($size_2m - 1);
+
+	return $min + $offset;
+}
+
+1;
diff --git a/scripts/aa64-load-offset.pl b/scripts/aa64-load-offset.pl
new file mode 100755
index 0000000..664b750
--- /dev/null
+++ b/scripts/aa64-load-offset.pl
@@ -0,0 +1,28 @@
+#!/usr/bin/perl -w
+# Find the load address of an AArch64 Linux Image
+#
+# Usage: ./$0 <Image> <min-offset>
+#
+# Copyright (C) 2021 ARM Limited. All rights reserved.
+#
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE.txt file.
+
+use warnings;
+use strict;
+
+use AA64Image;
+
+my $filename = shift;
+die("No filename provided") unless defined($filename);
+
+my $min = shift;
+$min = oct($min) if $min =~ /^0/;
+
+open (my $fh, "<:raw", $filename) or die("Unable to open file '$filename'");
+
+my $image = AA64Image->parse($fh) or die("Unable to parse Image");
+
+my $offset = $image->get_load_offset($min);
+
+printf("0x%016x\n", $offset);
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 08/12] Consistently use logical CPU IDs
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
                   ` (6 preceding siblings ...)
  2021-07-29 15:20 ` [boot-wrapper PATCH 07/12] aarch64: respect text offset Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 17:38   ` Andre Przywara
  2021-07-29 15:20 ` [boot-wrapper PATCH 09/12] Cleanup `.globl` usage Mark Rutland
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

In some places we assume that the cpu with MPIDR_EL1.Aff* == 0 is the
same as logical CPU 0. While this is almost certainly true, it would be
best to consistently use the ligical ID.

Add a new `this_cpu_logical_id()` helper, and use this in preference to
checking the MPIDR_EL1.Aff* bits directly.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 gic-v3.c      | 4 +---
 gic.c         | 3 +--
 include/cpu.h | 2 ++
 psci.c        | 5 ++---
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/gic-v3.c b/gic-v3.c
index ae2d2bc..62f9676 100644
--- a/gic-v3.c
+++ b/gic-v3.c
@@ -101,8 +101,6 @@ void gic_secure_init_primary(void)
 
 void gic_secure_init(void)
 {
-	uint32_t cpu = read_mpidr();
-
 	uint32_t sre;
 
 	/*
@@ -113,7 +111,7 @@ void gic_secure_init(void)
 	if (!has_gicv3_sysreg())
 		return;
 
-	if (cpu == 0)
+	if (this_cpu_logical_id() == 0)
 		gic_secure_init_primary();
 
 	sre = gic_read_icc_sre();
diff --git a/gic.c b/gic.c
index a84779e..04d4289 100644
--- a/gic.c
+++ b/gic.c
@@ -31,14 +31,13 @@ void gic_secure_init(void)
 {
 	unsigned int i;
 
-	uint32_t cpu = read_mpidr();
 	void *gicd_base = (void *)GIC_DIST_BASE;
 	void *gicc_base = (void *)GIC_CPU_BASE;
 
 	/* Set local interrupts to Group 1 (those fields are banked) */
 	raw_writel(~0, gicd_base + GICD_IGROUPRn);
 
-	if (cpu == 0) {
+	if (this_cpu_logical_id() == 0) {
 		uint32_t typer = raw_readl(gicd_base + GICD_TYPER);
 
 		/* Set SPIs to Group 1 */
diff --git a/include/cpu.h b/include/cpu.h
index 704d6d8..4bfb172 100644
--- a/include/cpu.h
+++ b/include/cpu.h
@@ -25,5 +25,7 @@
 
 unsigned int find_logical_id(unsigned long mpidr);
 
+#define this_cpu_logical_id()	find_logical_id(read_mpidr())
+
 #endif /* !__ASSEMBLY__ */
 #endif
diff --git a/psci.c b/psci.c
index e5d54b7..df5b0f2 100644
--- a/psci.c
+++ b/psci.c
@@ -35,7 +35,7 @@ static int psci_cpu_on(unsigned long target_mpidr, unsigned long address)
 {
 	int ret;
 	unsigned int cpu = find_logical_id(target_mpidr);
-	unsigned int this_cpu = find_logical_id(read_mpidr());
+	unsigned int this_cpu = this_cpu_logical_id();
 
 	if (cpu == MPIDR_INVALID)
 		return PSCI_RET_INVALID_PARAMETERS;
@@ -49,8 +49,7 @@ static int psci_cpu_on(unsigned long target_mpidr, unsigned long address)
 
 static int psci_cpu_off(void)
 {
-	unsigned long mpidr = read_mpidr();
-	unsigned int cpu = find_logical_id(mpidr);
+	unsigned int cpu = this_cpu_logical_id();
 
 	if (cpu == MPIDR_INVALID)
 		return PSCI_RET_DENIED;
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 09/12] Cleanup `.globl` usage
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
                   ` (7 preceding siblings ...)
  2021-07-29 15:20 ` [boot-wrapper PATCH 08/12] Consistently use logical CPU IDs Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 17:39   ` Andre Przywara
  2021-07-29 15:20 ` [boot-wrapper PATCH 10/12] aarch32: rename `_spin_dead` -> `err_invalid_id` Mark Rutland
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

In some places we use `ENTRY()` to mark an assembly function, whereas in
others we just use `.globl`. Where we use `.globl` for functions or
data, we don't keep this close to the actual definition.

Further, `ENTRY()` is a keyword in linker script with a different
meaning, and so it would be nicer if we didn't use the same term in the
assembly files.

This patch adds `ASM_FUNC()` and `ASM_DATA()` markers, and uses them
consistently throughout the codebase.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch32/boot.S  |  4 ++--
 arch/aarch32/psci.S  |  4 ++--
 arch/aarch32/stack.S | 11 +++--------
 arch/aarch32/utils.S |  4 ++--
 arch/aarch64/boot.S  |  8 +++-----
 arch/aarch64/psci.S  |  9 +++------
 arch/aarch64/spin.S  |  8 +++-----
 arch/aarch64/stack.S | 11 ++++-------
 arch/aarch64/utils.S |  9 +++------
 include/linkage.h    |  6 +++++-
 10 files changed, 30 insertions(+), 44 deletions(-)

diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index 0bd1ca2..15e51d4 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -16,7 +16,7 @@
 	.arch_extension virt
 
 	.section .init
-ENTRY(_start)
+ASM_FUNC(_start)
 	/* Stack initialisation */
 	cpuid	r0, r1
 	bl	find_logical_id
@@ -67,7 +67,7 @@ _spin_dead:
 	 * r0: kernel address
 	 * r1-r3, sp[0]: kernel arguments
 	 */
-ENTRY(jump_kernel)
+ASM_FUNC(jump_kernel)
 	sub	sp, #4				@ Ignore fourth argument
 	push	{r0 - r3}
 	mov	r5, sp
diff --git a/arch/aarch32/psci.S b/arch/aarch32/psci.S
index 0b7663e..dc7aeb7 100644
--- a/arch/aarch32/psci.S
+++ b/arch/aarch32/psci.S
@@ -38,12 +38,12 @@ handle_smc:
 	pop	{r4 - r12, lr}
 	movs	pc, lr
 
-ENTRY(start_el3)
+ASM_FUNC(start_el3)
 	ldr	r0, =smc_vectors
 	blx	setup_vector
 	/* pass through */
 
-ENTRY(start_no_el3)
+ASM_FUNC(start_no_el3)
 	/*
 	 * For no-el3, we assume that firmware launched the boot-wrapper in
 	 * non-secure EL2 or EL1. We assume it has its own PSCI implementation
diff --git a/arch/aarch32/stack.S b/arch/aarch32/stack.S
index 59f3f52..ac6885b 100644
--- a/arch/aarch32/stack.S
+++ b/arch/aarch32/stack.S
@@ -6,18 +6,13 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE.txt file.
  */
-
-	.globl setup_stack
-	.globl stack_top
-	.globl stack_bottom
-
 	.text
 	/*
 	 * Setup initial stack pointer
 	 * r0: logical CPU ID
 	 * Clobbers r1 and r2
 	 */
-setup_stack:
+ASM_FUNC(setup_stack)
 	mov	r1, #STACK_SIZE
 	ldr	r2, =stack_top
 	mls	sp, r0, r1, r2
@@ -25,8 +20,8 @@ setup_stack:
 
 	.section .stack
 	.align 2
-stack_bottom:
+ASM_DATA(stack_bottom)
 	.irp cpu, CPU_IDS
 	.space STACK_SIZE
 	.endr
-stack_top:
+ASM_DATA(stack_top)
diff --git a/arch/aarch32/utils.S b/arch/aarch32/utils.S
index 53d8747..5809f48 100644
--- a/arch/aarch32/utils.S
+++ b/arch/aarch32/utils.S
@@ -18,7 +18,7 @@
  * Returns MPIDR_INVALID for unknown MPIDRs
  * Clobbers r1, r2, r3.
  */
-ENTRY(find_logical_id)
+ASM_FUNC(find_logical_id)
 	ldr	r2, =id_table
 	mov	r1, #0
 1:	mov	r3, #NR_CPUS
@@ -40,7 +40,7 @@ ENTRY(find_logical_id)
  * Setup EL3 vectors.
  * r0: vector address
  */
-ENTRY(setup_vector)
+ASM_FUNC(setup_vector)
 	mcr	p15, 0, r0, c12, c0, 1	@ MVBAR
 	isb
 	bx	lr
diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index fae0188..ca73bcd 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -6,15 +6,13 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE.txt file.
  */
+#include <linkage.h>
 
 #include "common.S"
 
 	.section .init
 
-	.globl	_start
-	.globl jump_kernel
-
-_start:
+ASM_FUNC(_start)
 	cpuid	x0, x1
 	bl	find_logical_id
 	cmp	x0, #MPIDR_INVALID
@@ -119,7 +117,7 @@ err_invalid_id:
 	 * x0:		entry address
 	 * x1-x4:	arguments
 	 */
-jump_kernel:
+ASM_FUNC(jump_kernel)
 	mov	x19, x0
 	mov	x20, x1
 	mov	x21, x2
diff --git a/arch/aarch64/psci.S b/arch/aarch64/psci.S
index 6dbca11..8bd224b 100644
--- a/arch/aarch64/psci.S
+++ b/arch/aarch64/psci.S
@@ -6,6 +6,7 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE.txt file.
  */
+#include <linkage.h>
 #include <psci.h>
 
 #include "common.S"
@@ -45,9 +46,6 @@ vector:
 
 	.text
 
-	.globl start_no_el3
-	.globl start_el3
-
 err_exception:
 	b err_exception
 
@@ -81,8 +79,7 @@ smc_exit:
 	ldp	x18, x19, [sp], #16
 	eret
 
-
-start_el3:
+ASM_FUNC(start_el3)
 	ldr	x0, =vector
 	bl	setup_vector
 
@@ -95,7 +92,7 @@ start_el3:
  * This PSCI implementation requires EL3. Without EL3 we'll only boot the
  * primary cpu, all others will be trapped in an infinite loop.
  */
-start_no_el3:
+ASM_FUNC(start_no_el3)
 	cpuid	x0, x1
 	bl	find_logical_id
 	cbz	x0, psci_first_spin
diff --git a/arch/aarch64/spin.S b/arch/aarch64/spin.S
index ca05937..1ea1c0b 100644
--- a/arch/aarch64/spin.S
+++ b/arch/aarch64/spin.S
@@ -6,16 +6,14 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE.txt file.
  */
+#include <linkage.h>
 
 #include "common.S"
 
 	.text
 
-	.globl start_no_el3
-	.globl start_el3
-
-start_el3:
-start_no_el3:
+ASM_FUNC(start_el3)
+ASM_FUNC(start_no_el3)
 	cpuid	x0, x1
 	bl	find_logical_id
 
diff --git a/arch/aarch64/stack.S b/arch/aarch64/stack.S
index 8fb38ba..c89c388 100644
--- a/arch/aarch64/stack.S
+++ b/arch/aarch64/stack.S
@@ -6,10 +6,7 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE.txt file.
  */
-
-	.globl setup_stack
-	.globl stack_top
-	.globl stack_bottom
+#include <linkage.h>
 
 	.text
 	/*
@@ -17,7 +14,7 @@
 	 * x0: logical CPU ID
 	 * Clobbers x1 and x2
 	 */
-setup_stack:
+ASM_FUNC(setup_stack)
 	mov	w1, #STACK_SIZE
 	ldr	x2, =stack_top
 	umsubl	x0, w0, w1, x2			// sp = st_base - cpu * st_size
@@ -26,8 +23,8 @@ setup_stack:
 
 	.section .stack
 	.align 4
-stack_bottom:
+ASM_DATA(stack_bottom)
 	.irp cpu, CPU_IDS
 	.space STACK_SIZE
 	.endr
-stack_top:
+ASM_DATA(stack_top)
diff --git a/arch/aarch64/utils.S b/arch/aarch64/utils.S
index ae22ea7..85c7f8a 100644
--- a/arch/aarch64/utils.S
+++ b/arch/aarch64/utils.S
@@ -6,11 +6,8 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE.txt file.
  */
-
 #include <cpu.h>
-
-	.globl find_logical_id
-	.globl setup_vector
+#include <linkage.h>
 
 	.text
 
@@ -20,7 +17,7 @@
  * Sets the Z flag when CPU is primary
  * Clobbers x1, x2, x3
  */
-find_logical_id:
+ASM_FUNC(find_logical_id)
 	ldr	x2, =id_table
 	mov	x1, xzr
 1:	mov	x3, #NR_CPUS	// check we haven't walked off the end of the array
@@ -40,7 +37,7 @@ find_logical_id:
  * Setup EL3 vectors
  * x0: vector address
  */
-setup_vector:
+ASM_FUNC(setup_vector)
 	msr	VBAR_EL3, x0
 	isb
 	ret
diff --git a/include/linkage.h b/include/linkage.h
index 844a811..db8b204 100644
--- a/include/linkage.h
+++ b/include/linkage.h
@@ -13,10 +13,14 @@
 
 #ifdef __ASSEMBLY__
 
-#define ENTRY(name)				\
+#define ASM_FUNC(name)				\
 	.globl name;				\
 	.type  name, %function;			\
 	name:
 
+#define ASM_DATA(name)				\
+	.globl name;				\
+	name:
+
 #endif /* __ASSEMBLY__ */
 #endif
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 10/12] aarch32: rename `_spin_dead` -> `err_invalid_id`
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
                   ` (8 preceding siblings ...)
  2021-07-29 15:20 ` [boot-wrapper PATCH 09/12] Cleanup `.globl` usage Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 17:39   ` Andre Przywara
  2021-07-29 15:20 ` [boot-wrapper PATCH 11/12] Rename `spin.h` -> `boot.h` Mark Rutland
  2021-07-29 15:20 ` [boot-wrapper PATCH 12/12] Move common source files to `common` directory Mark Rutland
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

For clarity, align aarch32 with aarch64, sending unexpected CPUs to an
`err_invalid_id` loop rather than `_spin_dead`.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch32/boot.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index 15e51d4..9e5c308 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -21,7 +21,7 @@ ASM_FUNC(_start)
 	cpuid	r0, r1
 	bl	find_logical_id
 	cmp	r0, #MPIDR_INVALID
-	beq	_spin_dead
+	beq	err_invalid_id
 
 	bl	setup_stack
 
@@ -58,7 +58,7 @@ _monitor:
 	/* Initialise boot method */
 	b	start_el3
 
-_spin_dead:
+err_invalid_id:
 	b	.
 
 	.text
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 11/12] Rename `spin.h` -> `boot.h`
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
                   ` (9 preceding siblings ...)
  2021-07-29 15:20 ` [boot-wrapper PATCH 10/12] aarch32: rename `_spin_dead` -> `err_invalid_id` Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 17:39   ` Andre Przywara
  2021-07-29 15:20 ` [boot-wrapper PATCH 12/12] Move common source files to `common` directory Mark Rutland
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

In `spin.h` we have function prototypes provided by `boot.c`. For
clarity, let's align the naming.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 boot_common.c              | 2 +-
 include/{spin.h => boot.h} | 6 +++---
 psci.c                     | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
 rename include/{spin.h => boot.h} (88%)

diff --git a/boot_common.c b/boot_common.c
index d48b7e1..eada179 100644
--- a/boot_common.c
+++ b/boot_common.c
@@ -6,8 +6,8 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE.txt file.
  */
+#include <boot.h>
 #include <cpu.h>
-#include <spin.h>
 
 extern unsigned long entrypoint;
 extern unsigned long dtb;
diff --git a/include/spin.h b/include/boot.h
similarity index 88%
rename from include/spin.h
rename to include/boot.h
index 15bbe27..d75e013 100644
--- a/include/spin.h
+++ b/include/boot.h
@@ -1,13 +1,13 @@
 /*
- * include/spin.h
+ * include/boot.h
  *
  * Copyright (C) 2015 ARM Limited. All rights reserved.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE.txt file.
  */
-#ifndef __SPIN_H
-#define __SPIN_H
+#ifndef __BOOT_H
+#define __BOOT_H
 
 #include <compiler.h>
 
diff --git a/psci.c b/psci.c
index df5b0f2..a0e8700 100644
--- a/psci.c
+++ b/psci.c
@@ -10,9 +10,9 @@
 #include <stdint.h>
 
 #include <bakery_lock.h>
+#include <boot.h>
 #include <cpu.h>
 #include <psci.h>
-#include <spin.h>
 
 #ifndef CPU_IDS
 #error "No MPIDRs provided"
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [boot-wrapper PATCH 12/12] Move common source files to `common` directory
  2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
                   ` (10 preceding siblings ...)
  2021-07-29 15:20 ` [boot-wrapper PATCH 11/12] Rename `spin.h` -> `boot.h` Mark Rutland
@ 2021-07-29 15:20 ` Mark Rutland
  2021-07-30 17:40   ` Andre Przywara
  11 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-29 15:20 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Andre.Przywara, Jaxson.Han, mark.rutland, Wei.Chen

The top-level directory is getting increasingly cluttered. For clarity
let's move the common source files into their own directory. At the same
time let's clean up the way we generate object lists so that it's
consistent for arch/common objects, and doesn't require special casing
each optional object.

Note that we also need to create a common/ directory for out-of-tree
builds.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 Makefile.am                           | 29 +++++++++++++++++------------
 bakery_lock.c => common/bakery_lock.c |  0
 boot_common.c => common/boot.c        |  2 +-
 gic-v3.c => common/gic-v3.c           |  0
 gic.c => common/gic.c                 |  0
 lib.c => common/lib.c                 |  0
 platform.c => common/platform.c       |  0
 psci.c => common/psci.c               |  0
 8 files changed, 18 insertions(+), 13 deletions(-)
 rename bakery_lock.c => common/bakery_lock.c (100%)
 rename boot_common.c => common/boot.c (96%)
 rename gic-v3.c => common/gic-v3.c (100%)
 rename gic.c => common/gic.c (100%)
 rename lib.c => common/lib.c (100%)
 rename platform.c => common/platform.c (100%)
 rename psci.c => common/psci.c (100%)

diff --git a/Makefile.am b/Makefile.am
index 5d34cc8..68f23d3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -33,7 +33,10 @@ PSCI_CPU_ON	:= 0xc4000003
 endif
 PSCI_CPU_OFF	:= 0x84000002
 
-OFILES		=
+COMMON_SRC	:= common/
+COMMON_OBJ	:= boot.o bakery_lock.o platform.o lib.o
+
+ARCH_OBJ	:= boot.o stack.o utils.o
 
 if BOOTWRAPPER_32
 CPPFLAGS	+= -DBOOTWRAPPER_32
@@ -45,8 +48,8 @@ ARCH_SRC	:= arch/aarch64/
 endif
 
 if PSCI
-BOOTMETHOD	:= psci.o
-OFILES		+= psci.o
+ARCH_OBJ	+= psci.o
+COMMON_OBJ	+= psci.o
 PSCI_NODE	:= psci {				\
 			compatible = \"arm,psci\";	\
 			method = \"smc\";		\
@@ -55,7 +58,7 @@ PSCI_NODE	:= psci {				\
 		   };
 CPU_NODES	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/addpsci.pl $(KERNEL_DTB))
 else
-BOOTMETHOD	:= spin.o
+ARCH_OBJ	+= spin.o
 PSCI_NODE	:=
 CPU_NODES	:=
 endif
@@ -65,13 +68,13 @@ GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNE
 GIC_RDIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3')
 DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
 DEFINES		+= -DGIC_RDIST_BASE=$(GIC_RDIST_BASE)
-GIC		:= gic-v3.o
+COMMON_OBJ	+= gic-v3.o
 else
 GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic')
 GIC_CPU_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic')
 DEFINES		+= -DGIC_CPU_BASE=$(GIC_CPU_BASE)
 DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
-GIC		:= gic.o
+COMMON_OBJ	+= gic.o
 endif
 
 if KERNEL_32
@@ -125,8 +128,7 @@ CFLAGS		+= -Wall -fomit-frame-pointer
 CFLAGS		+= -ffunction-sections -fdata-sections
 LDFLAGS		+= --gc-sections
 
-OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) lib.o
-OFILES		+= $(addprefix $(ARCH_SRC),boot.o stack.o $(BOOTMETHOD) utils.o)
+OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
 
 # Don't lookup all prerequisites in $(top_srcdir), only the source files. When
 # building outside the source tree $(ARCH_SRC) needs to be created.
@@ -136,18 +138,21 @@ vpath %.S $(top_srcdir)
 
 all: $(IMAGE)
 
-CLEANFILES = $(IMAGE) linux-system.axf xen-system.axf $(OFILES) model.lds fdt.dtb
+CLEANFILES = $(IMAGE) linux-system.axf xen-system.axf $(OBJ) model.lds fdt.dtb
 
-$(IMAGE): $(OFILES) model.lds fdt.dtb $(KERNEL_IMAGE) $(FILESYSTEM) $(XEN_IMAGE)
-	$(LD) $(LDFLAGS) $(OFILES) -o $@ --script=model.lds
+$(IMAGE): $(OBJ) model.lds fdt.dtb $(KERNEL_IMAGE) $(FILESYSTEM) $(XEN_IMAGE)
+	$(LD) $(LDFLAGS) $(OBJ) -o $@ --script=model.lds
 
 $(ARCH_SRC):
 	$(MKDIR_P) $@
 
+$(COMMON_SRC):
+	$(MKDIR_P) $@
+
 %.o: %.S Makefile | $(ARCH_SRC)
 	$(CC) $(CPPFLAGS) -D__ASSEMBLY__ $(CFLAGS) $(DEFINES) -c -o $@ $<
 
-%.o: %.c Makefile
+%.o: %.c Makefile | $(COMMON_SRC)
 	$(CC) $(CPPFLAGS) $(CFLAGS) $(DEFINES) -c -o $@ $<
 
 model.lds: $(LD_SCRIPT) Makefile
diff --git a/bakery_lock.c b/common/bakery_lock.c
similarity index 100%
rename from bakery_lock.c
rename to common/bakery_lock.c
diff --git a/boot_common.c b/common/boot.c
similarity index 96%
rename from boot_common.c
rename to common/boot.c
index eada179..c74d34c 100644
--- a/boot_common.c
+++ b/common/boot.c
@@ -1,5 +1,5 @@
 /*
- * boot_common.c - common spin function for all boot methods
+ * boot.c - common spin function for all boot methods
  *
  * Copyright (C) 2015 ARM Limited. All rights reserved.
  *
diff --git a/gic-v3.c b/common/gic-v3.c
similarity index 100%
rename from gic-v3.c
rename to common/gic-v3.c
diff --git a/gic.c b/common/gic.c
similarity index 100%
rename from gic.c
rename to common/gic.c
diff --git a/lib.c b/common/lib.c
similarity index 100%
rename from lib.c
rename to common/lib.c
diff --git a/platform.c b/common/platform.c
similarity index 100%
rename from platform.c
rename to common/platform.c
diff --git a/psci.c b/common/psci.c
similarity index 100%
rename from psci.c
rename to common/psci.c
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 01/12] Ensure `kernel_address` is aligned
  2021-07-29 15:20 ` [boot-wrapper PATCH 01/12] Ensure `kernel_address` is aligned Mark Rutland
@ 2021-07-30 15:11   ` Andre Przywara
  0 siblings, 0 replies; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 15:11 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:39 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> We accidentally placed the `.align` directive after the `kernel_address`
> label, meaning that the label itself isn't necessarily aligned. Place
> the `.align` directive first to ensure this.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Ouch, good catch!

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arch/aarch64/spin.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/aarch64/spin.S b/arch/aarch64/spin.S
> index 72603cf..ca05937 100644
> --- a/arch/aarch64/spin.S
> +++ b/arch/aarch64/spin.S
> @@ -30,8 +30,8 @@ start_no_el3:
>  	mov	x2, #0
>  	bl	first_spin
>  
> -kernel_address:
>  	.align 3
> +kernel_address:
>  	.long 0
>  
>  	.ltorg


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 03/12] Remove cache maintenance
  2021-07-29 15:20 ` [boot-wrapper PATCH 03/12] Remove cache maintenance Mark Rutland
@ 2021-07-30 15:12   ` Andre Przywara
  2021-07-30 15:43     ` Mark Rutland
  0 siblings, 1 reply; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 15:12 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:41 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> For models, we assume that out-of-reset caches ar invalid and no cache

Is that "caches are invalid out-of-reset" an architecture guarantee or
just a model property? Since the bootwrapper is officially targeting
the model, that doesn't really matter, but some people (ab)use it for
other platforms, so it might be worth noting.

> maintenance is required.
> 
> We added cache maintenance to the boot-wrapper in commit:
> 
>   28ec269a22c8dc14 ("Add code to clean and invalidate caches")
> 
> ... because the boot-wrapper would teansiently use cacheable mappings,
> and could allocate into caches. As we were using Set/Way operations, we
> were on somewhat shaky ground (e.g. due to system-level caches, or
> dirty line migration). Further, we never took FEAT_CCIDX into account,
> and so would not necessarily invalidate all potential levels of cache
> 
> However, since commit:
> 
>   0bb7b2545582accf ("Remove MMU identity map setup")
> 
> ... we no longer enable the MMU within the boot-wrapper, and so no
> longer have any reason to perform cache maintenance.
> 
> This patch removes the redundant and incomplete cache maintenance.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  Makefile.am   |  2 +-
>  boot_common.c |  3 ---
>  cache.c       | 58 ----------------------------------------------------------

Love that ^^^^, also it removes the *flush*_cache() name ;-)

Indeed I couldn't find anything setting SCTLR.M, so:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

>  3 files changed, 1 insertion(+), 62 deletions(-)
>  delete mode 100644 cache.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index ef6b793..8334049 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -123,7 +123,7 @@ CFLAGS		+= -Wall -fomit-frame-pointer
>  CFLAGS		+= -ffunction-sections -fdata-sections
>  LDFLAGS		+= --gc-sections
>  
> -OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) cache.o lib.o
> +OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) lib.o
>  OFILES		+= $(addprefix $(ARCH_SRC),boot.o stack.o $(BOOTMETHOD) utils.o)
>  
>  # Don't lookup all prerequisites in $(top_srcdir), only the source files. When
> diff --git a/boot_common.c b/boot_common.c
> index e7b8e1d..d48b7e1 100644
> --- a/boot_common.c
> +++ b/boot_common.c
> @@ -13,7 +13,6 @@ extern unsigned long entrypoint;
>  extern unsigned long dtb;
>  
>  void init_platform(void);
> -void flush_caches(void);
>  
>  void __noreturn jump_kernel(unsigned long address,
>  			    unsigned long a0,
> @@ -62,8 +61,6 @@ void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry)
>  void __noreturn first_spin(unsigned int cpu, unsigned long *mbox,
>  			   unsigned long invalid)
>  {
> -	flush_caches();
> -
>  	if (cpu == 0) {
>  		init_platform();
>  
> diff --git a/cache.c b/cache.c
> deleted file mode 100644
> index 9d71248..0000000
> --- a/cache.c
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -/*
> - * cache.c - simple cache clean+invalidate code
> - *
> - * Copyright (C) 2015 ARM Limited. All rights reserved.
> - *
> - * Use of this source code is governed by a BSD-style license that can be
> - * found in the LICENSE.txt file.
> - */
> -
> -#include <cpu.h>
> -
> -void flush_caches(void)
> -{
> -	unsigned int level;
> -	uint32_t clidr = read_clidr();
> -	unsigned int max_level = (clidr >> 24) & 0x7;
> -
> -	uint32_t ccsidr;
> -
> -	if (max_level == 0)
> -		return;
> -
> -	for (level = 0; level < max_level; level++) {
> -		uint32_t cache_type = (clidr >> (level * 3)) & 0x7;
> -		unsigned int line_size, num_ways, num_sets, way_shift;
> -		unsigned int way, set;
> -
> -		if (cache_type == 1)
> -			/* No dcache at this level */
> -			continue;
> -
> -		write_csselr(level << 1);
> -		isb();
> -		ccsidr = read_ccsidr();
> -
> -		line_size = (ccsidr & 0x7) + 4; /* log2 line size */
> -		num_ways = ((ccsidr >> 3) & 0x3ff) + 1;
> -		num_sets = ((ccsidr >> 13) & 0x7fff) + 1;
> -
> -		way_shift = clz(num_ways - 1);
> -		for (way = 0; way < num_ways; way++) {
> -			for (set = 0; set < num_sets; set++) {
> -				uint32_t command = level << 1;
> -				command |= way << way_shift;
> -				command |= set << line_size;
> -
> -				dccisw(command);
> -				dsb(sy);
> -			}
> -		}
> -
> -		dsb(sy);
> -	}
> -	dsb(sy);
> -	iciallu();
> -	dsb(sy);
> -	isb();
> -}


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 04/12] Remove `flag_no_el3`
  2021-07-29 15:20 ` [boot-wrapper PATCH 04/12] Remove `flag_no_el3` Mark Rutland
@ 2021-07-30 15:13   ` Andre Przywara
  2021-07-30 16:43     ` Mark Rutland
  0 siblings, 1 reply; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 15:13 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:42 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

Hi,

> We set `flag_no_el3` when not booted at EL3 / monitor mode, and
> subsequently we use this to determine whether we need to drop exception
> level before entering Linux. As this can be derived from CurrentEL or
> CPSR, the flag itself is redundant, and we can defer the check until
> we're about to enter Linux.
> 
> In future this will allow more logic to be converted into C, where it
> will be easier to handle architectural variants.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/aarch32/boot.S | 14 +++-----------
>  arch/aarch64/boot.S | 13 ++-----------
>  2 files changed, 5 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> index 2a85ad5..0bd1ca2 100644
> --- a/arch/aarch32/boot.S
> +++ b/arch/aarch32/boot.S
> @@ -31,9 +31,6 @@ ENTRY(_start)
>  	cmp	r0, #PSR_HYP
>  	bne	_switch_monitor

Can't this become "beq start_no_el3" now?

>  
> -	mov	r0, #1
> -	ldr	r1, =flag_no_el3
> -	str	r0, [r1]
>  	b	start_no_el3
>  
>  _switch_monitor:
> @@ -89,9 +86,9 @@ ENTRY(jump_kernel)
>  	ldr	lr, [r5], #4
>  	ldm	r5, {r0 - r2}
>  
> -	ldr	r4, =flag_no_el3
> -	ldr	r4, [r4]
> -	cmp	r4, #1
> +	mrs	r4, cpsr
> +	and	r4, #PSR_MODE_MASK
> +	cmp	r4, #PSR_MON

Is comparing explicitly against monitor mode the right thing? IIRC
normally we come out of reset in secure SVC, and this *is* EL3 (the
highest implemented exception level), from an ARMv8 perspective.
The old code did compare against HYP, which is probably what we want
and is also one of the few modes we are sure of being not EL3.

>  	bxeq	lr				@ no EL3
>  
>  	ldr	r4, =SPSR_KERNEL
> @@ -113,8 +110,3 @@ boot_vectors:
>  	b	.
>  	b	.
>  	b	.
> -
> -	.section .data
> -	.align 2
> -flag_no_el3:
> -	.long 0
> diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> index 37759ce..fae0188 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -28,10 +28,6 @@ _start:
>  	cmp	x0, #CURRENTEL_EL3
>  	b.eq	1f

Can't this become "b.ne start_no_el3" now?

Cheers,
Andre

>  
> -	mov	w0, #1
> -	ldr	x1, =flag_no_el3
> -	str	w0, [x1]
> -
>  	b	start_no_el3
>  
>  1:	mov	x0, #0x30			// RES1
> @@ -140,8 +136,8 @@ jump_kernel:
>  	bl	find_logical_id
>  	bl	setup_stack		// Reset stack pointer
>  
> -	ldr	w0, flag_no_el3
> -	cmp	w0, #0			// Prepare Z flag
> +	mrs	x0, CurrentEl
> +	cmp	w0, #CURRENTEL_EL3	// Prepare Z flag
>  
>  	mov	x0, x20
>  	mov	x1, x21
> @@ -164,8 +160,3 @@ jump_kernel:
>  	eret
>  
>  	.ltorg
> -
> -	.data
> -	.align 3
> -flag_no_el3:
> -	.long 0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 06/12] Move scripts to a `scripts` directory
  2021-07-29 15:20 ` [boot-wrapper PATCH 06/12] Move scripts to a `scripts` directory Mark Rutland
@ 2021-07-30 15:13   ` Andre Przywara
  0 siblings, 0 replies; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 15:13 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:44 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> The top-level directory is getting increasingly cluttered. For clarity
> let's move the scripts into their own directory.
> 
> There should be no functional change as a result of this patch.

Yes, confirmed to be just renaming/moving. Requires autoreconf -i
though, of course.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  Makefile.am                            | 20 +++++++++++---------
>  FDT.pm => scripts/FDT.pm               |  0
>  addpsci.pl => scripts/addpsci.pl       |  0
>  findbase.pl => scripts/findbase.pl     |  0
>  findcpuids.pl => scripts/findcpuids.pl |  0
>  findmem.pl => scripts/findmem.pl       |  0
>  6 files changed, 11 insertions(+), 9 deletions(-)
>  rename FDT.pm => scripts/FDT.pm (100%)
>  rename addpsci.pl => scripts/addpsci.pl (100%)
>  rename findbase.pl => scripts/findbase.pl (100%)
>  rename findcpuids.pl => scripts/findcpuids.pl (100%)
>  rename findmem.pl => scripts/findmem.pl (100%)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 8334049..ad13dbc 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -7,13 +7,15 @@
>  # Use of this source code is governed by a BSD-style license that can be
>  # found in the LICENSE.txt file.
>  
> +SCRIPT_DIR	:= $(top_srcdir)/scripts
> +
>  # VE
> -PHYS_OFFSET	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findmem.pl $(KERNEL_DTB))
> -UART_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 0 'arm,pl011')
> -SYSREGS_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 0 'arm,vexpress-sysreg' 2> /dev/null)
> +PHYS_OFFSET	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findmem.pl $(KERNEL_DTB))
> +UART_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,pl011')
> +SYSREGS_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,vexpress-sysreg' 2> /dev/null)
>  CNTFRQ		:= 0x01800000	# 24Mhz
>  
> -CPU_IDS		:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findcpuids.pl $(KERNEL_DTB))
> +CPU_IDS		:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findcpuids.pl $(KERNEL_DTB))
>  NR_CPUS         := $(shell echo $(CPU_IDS) | tr ',' ' ' | wc -w)
>  
>  DEFINES		= -DCNTFRQ=$(CNTFRQ)
> @@ -51,7 +53,7 @@ PSCI_NODE	:= psci {				\
>  			cpu_on = <$(PSCI_CPU_ON)>;	\
>  			cpu_off = <$(PSCI_CPU_OFF)>;	\
>  		   };
> -CPU_NODES	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/addpsci.pl $(KERNEL_DTB))
> +CPU_NODES	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/addpsci.pl $(KERNEL_DTB))
>  else
>  BOOTMETHOD	:= spin.o
>  PSCI_NODE	:=
> @@ -59,14 +61,14 @@ CPU_NODES	:=
>  endif
>  
>  if GICV3
> -GIC_DIST_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 0 'arm,gic-v3')
> -GIC_RDIST_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3')
> +GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,gic-v3')
> +GIC_RDIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3')
>  DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
>  DEFINES		+= -DGIC_RDIST_BASE=$(GIC_RDIST_BASE)
>  GIC		:= gic-v3.o
>  else
> -GIC_DIST_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic')
> -GIC_CPU_BASE	:= $(shell perl -I $(top_srcdir) $(top_srcdir)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic')
> +GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic')
> +GIC_CPU_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic')
>  DEFINES		+= -DGIC_CPU_BASE=$(GIC_CPU_BASE)
>  DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
>  GIC		:= gic.o
> diff --git a/FDT.pm b/scripts/FDT.pm
> similarity index 100%
> rename from FDT.pm
> rename to scripts/FDT.pm
> diff --git a/addpsci.pl b/scripts/addpsci.pl
> similarity index 100%
> rename from addpsci.pl
> rename to scripts/addpsci.pl
> diff --git a/findbase.pl b/scripts/findbase.pl
> similarity index 100%
> rename from findbase.pl
> rename to scripts/findbase.pl
> diff --git a/findcpuids.pl b/scripts/findcpuids.pl
> similarity index 100%
> rename from findcpuids.pl
> rename to scripts/findcpuids.pl
> diff --git a/findmem.pl b/scripts/findmem.pl
> similarity index 100%
> rename from findmem.pl
> rename to scripts/findmem.pl


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 07/12] aarch64: respect text offset
  2021-07-29 15:20 ` [boot-wrapper PATCH 07/12] aarch64: respect text offset Mark Rutland
@ 2021-07-30 15:13   ` Andre Przywara
  2021-07-30 15:43     ` Mark Rutland
  0 siblings, 1 reply; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 15:13 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:45 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> The boot-wrapper assumes that an AArch64 kernel's text offset is 0x80000
> rather than reading the `text_offset` field from the Image header as the
> documentation says it should.
> 
> Add a script to figure this out during the build process. As with FDT.pm
> the parsing of the Image (and common logic associated with this) is
> factored into a module that we may use in more scripts in future.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  Makefile.am                 |  4 +--
>  scripts/AA64Image.pm        | 87 +++++++++++++++++++++++++++++++++++++++++++++
>  scripts/aa64-load-offset.pl | 28 +++++++++++++++
>  3 files changed, 117 insertions(+), 2 deletions(-)
>  create mode 100755 scripts/AA64Image.pm
>  create mode 100755 scripts/aa64-load-offset.pl
> 
> diff --git a/Makefile.am b/Makefile.am
> index ad13dbc..5d34cc8 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -76,12 +76,12 @@ endif
>  
>  if KERNEL_32
>  MBOX_OFFSET	:= 0x7ff8
> -KERNEL_OFFSET	:= 0x8000
>  TEXT_LIMIT	:= 0x3000
> +KERNEL_OFFSET	:= 0x8000
>  else
>  MBOX_OFFSET	:= 0xfff8
> -KERNEL_OFFSET	:= 0x80000
>  TEXT_LIMIT	:= 0x80000
> +KERNEL_OFFSET	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/aa64-load-offset.pl $(KERNEL_IMAGE) $(TEXT_LIMIT))
>  endif
>  
>  LD_SCRIPT	:= model.lds.S
> diff --git a/scripts/AA64Image.pm b/scripts/AA64Image.pm
> new file mode 100755
> index 0000000..36b2547
> --- /dev/null
> +++ b/scripts/AA64Image.pm
> @@ -0,0 +1,87 @@
> +#!/usr/bin/perl -w
> +# A simple Flattened Device Tree Blob (FDT/DTB) parser.
> +#
> +# Copyright (C) 2014 ARM Limited. All rights reserved.
> +#
> +# Use of this source code is governed by a BSD-style license that can be
> +# found in the LICENSE.txt file.
> +
> +use warnings;
> +use strict;
> +use integer;
> +
> +package AA64Image;
> +
> +# Header definitions from v5.13
> +# See https://www.kernel.org/doc/html/v5.13/arm64/booting.html#call-the-kernel-image
> +
> +use constant {
> +	HEADER_LEN => 64,
> +	HEADER_MAGIC => 0x644d5241,
> +};
> +
> +sub parse
> +{
> +	my $class = shift;
> +	my $fh = shift;
> +	my $self = bless {}, $class;
> +
> +	read($fh, my $raw_header, AA64Image::HEADER_LEN) == AA64Image::HEADER_LEN or goto failed;
> +
> +	(
> +		$self->{code0},
> +		$self->{code1},
> +		$self->{text_offset},
> +		$self->{image_size},
> +		$self->{flags},
> +		$self->{res2},
> +		$self->{res3},
> +		$self->{res4},
> +		$self->{magic},
> +		$self->{res5}
> +	) = unpack("VVQ<Q<Q<Q<Q<Q<VV", $raw_header);
> +
> +	if ($self->{magic} != AA64Image::HEADER_MAGIC) {
> +		warn "Image header magic not found";
> +		goto failed;
> +	}
> +
> +	return $self;
> +
> +failed:
> +	warn "Unable to parse header";
> +	return undef;
> +}
> +
> +sub get_text_offset
> +{
> +	my $self = shift;
> +
> +	# Where image_size is 0, the load offset can be assumed to be 0x80000
> +	# See https://www.kernel.org/doc/html/v5.13/arm64/booting.html#call-the-kernel-image
> +	if ($self->{image_size} == 0) {
> +		return 0x80000;
> +	}
> +
> +	return $self->{text_offset};
> +}
> +
> +sub get_load_offset
> +{
> +	my $self = shift;
> +	my $min = shift;
> +	my $offset = $self->get_text_offset();
> +
> +	if ($min < $offset) {

So this should be ($min <= $offset), otherwise a kernel with
TEXT_OFFSET 0x80000 needlessly gets loaded up to 2.5MB.

Otherwise looks good and seems to handle the cases I tested correctly.

Cheers,
Andre

> +		return $offset;
> +	}
> +
> +	# The image must be placed text_offset bytes from a 2MB aligned base address
> +	my $size_2m = 2 * 1024 * 1024;
> +	$min += $size_2m - 1;
> +	$min &= ~($size_2m - 1);
> +
> +	return $min + $offset;
> +}
> +
> +1;
> diff --git a/scripts/aa64-load-offset.pl b/scripts/aa64-load-offset.pl
> new file mode 100755
> index 0000000..664b750
> --- /dev/null
> +++ b/scripts/aa64-load-offset.pl
> @@ -0,0 +1,28 @@
> +#!/usr/bin/perl -w
> +# Find the load address of an AArch64 Linux Image
> +#
> +# Usage: ./$0 <Image> <min-offset>
> +#
> +# Copyright (C) 2021 ARM Limited. All rights reserved.
> +#
> +# Use of this source code is governed by a BSD-style license that can be
> +# found in the LICENSE.txt file.
> +
> +use warnings;
> +use strict;
> +
> +use AA64Image;
> +
> +my $filename = shift;
> +die("No filename provided") unless defined($filename);
> +
> +my $min = shift;
> +$min = oct($min) if $min =~ /^0/;
> +
> +open (my $fh, "<:raw", $filename) or die("Unable to open file '$filename'");
> +
> +my $image = AA64Image->parse($fh) or die("Unable to parse Image");
> +
> +my $offset = $image->get_load_offset($min);
> +
> +printf("0x%016x\n", $offset);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 07/12] aarch64: respect text offset
  2021-07-30 15:13   ` Andre Przywara
@ 2021-07-30 15:43     ` Mark Rutland
  0 siblings, 0 replies; 27+ messages in thread
From: Mark Rutland @ 2021-07-30 15:43 UTC (permalink / raw)
  To: Andre Przywara; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Fri, Jul 30, 2021 at 04:13:38PM +0100, Andre Przywara wrote:
> On Thu, 29 Jul 2021 16:20:45 +0100
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > The boot-wrapper assumes that an AArch64 kernel's text offset is 0x80000
> > rather than reading the `text_offset` field from the Image header as the
> > documentation says it should.
> > 
> > Add a script to figure this out during the build process. As with FDT.pm
> > the parsing of the Image (and common logic associated with this) is
> > factored into a module that we may use in more scripts in future.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  Makefile.am                 |  4 +--
> >  scripts/AA64Image.pm        | 87 +++++++++++++++++++++++++++++++++++++++++++++
> >  scripts/aa64-load-offset.pl | 28 +++++++++++++++
> >  3 files changed, 117 insertions(+), 2 deletions(-)
> >  create mode 100755 scripts/AA64Image.pm
> >  create mode 100755 scripts/aa64-load-offset.pl

> > +sub get_load_offset
> > +{
> > +	my $self = shift;
> > +	my $min = shift;
> > +	my $offset = $self->get_text_offset();
> > +
> > +	if ($min < $offset) {
> 
> So this should be ($min <= $offset), otherwise a kernel with
> TEXT_OFFSET 0x80000 needlessly gets loaded up to 2.5MB.

Good spot; I've fixed that up locally.

> Otherwise looks good and seems to handle the cases I tested correctly.

Great!

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 03/12] Remove cache maintenance
  2021-07-30 15:12   ` Andre Przywara
@ 2021-07-30 15:43     ` Mark Rutland
  0 siblings, 0 replies; 27+ messages in thread
From: Mark Rutland @ 2021-07-30 15:43 UTC (permalink / raw)
  To: Andre Przywara; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Fri, Jul 30, 2021 at 04:12:23PM +0100, Andre Przywara wrote:
> On Thu, 29 Jul 2021 16:20:41 +0100
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > For models, we assume that out-of-reset caches ar invalid and no cache
> 
> Is that "caches are invalid out-of-reset" an architecture guarantee or
> just a model property? Since the bootwrapper is officially targeting
> the model, that doesn't really matter, but some people (ab)use it for
> other platforms, so it might be worth noting.

Just a model property; in general implementations can require an
IMPLEMENTATION DEFINED initialization sequence out-of-reset.

I'd like to document the assumptions the boot-wrapper makes, but at the
moment those aren't entirely consistent, so I'm holding back until teh
cleanup's done.

Practically speaking, it's unlikely people would build something weaker
as it would wreak havoc upon SMP and hotplug, etc.

> > maintenance is required.
> > 
> > We added cache maintenance to the boot-wrapper in commit:
> > 
> >   28ec269a22c8dc14 ("Add code to clean and invalidate caches")
> > 
> > ... because the boot-wrapper would teansiently use cacheable mappings,
> > and could allocate into caches. As we were using Set/Way operations, we
> > were on somewhat shaky ground (e.g. due to system-level caches, or
> > dirty line migration). Further, we never took FEAT_CCIDX into account,
> > and so would not necessarily invalidate all potential levels of cache
> > 
> > However, since commit:
> > 
> >   0bb7b2545582accf ("Remove MMU identity map setup")
> > 
> > ... we no longer enable the MMU within the boot-wrapper, and so no
> > longer have any reason to perform cache maintenance.
> > 
> > This patch removes the redundant and incomplete cache maintenance.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  Makefile.am   |  2 +-
> >  boot_common.c |  3 ---
> >  cache.c       | 58 ----------------------------------------------------------
> 
> Love that ^^^^, also it removes the *flush*_cache() name ;-)

:)

> Indeed I couldn't find anything setting SCTLR.M, so:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> Cheers,
> Andre

Thanks!

Mark.

> 
> >  3 files changed, 1 insertion(+), 62 deletions(-)
> >  delete mode 100644 cache.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index ef6b793..8334049 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -123,7 +123,7 @@ CFLAGS		+= -Wall -fomit-frame-pointer
> >  CFLAGS		+= -ffunction-sections -fdata-sections
> >  LDFLAGS		+= --gc-sections
> >  
> > -OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) cache.o lib.o
> > +OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) lib.o
> >  OFILES		+= $(addprefix $(ARCH_SRC),boot.o stack.o $(BOOTMETHOD) utils.o)
> >  
> >  # Don't lookup all prerequisites in $(top_srcdir), only the source files. When
> > diff --git a/boot_common.c b/boot_common.c
> > index e7b8e1d..d48b7e1 100644
> > --- a/boot_common.c
> > +++ b/boot_common.c
> > @@ -13,7 +13,6 @@ extern unsigned long entrypoint;
> >  extern unsigned long dtb;
> >  
> >  void init_platform(void);
> > -void flush_caches(void);
> >  
> >  void __noreturn jump_kernel(unsigned long address,
> >  			    unsigned long a0,
> > @@ -62,8 +61,6 @@ void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry)
> >  void __noreturn first_spin(unsigned int cpu, unsigned long *mbox,
> >  			   unsigned long invalid)
> >  {
> > -	flush_caches();
> > -
> >  	if (cpu == 0) {
> >  		init_platform();
> >  
> > diff --git a/cache.c b/cache.c
> > deleted file mode 100644
> > index 9d71248..0000000
> > --- a/cache.c
> > +++ /dev/null
> > @@ -1,58 +0,0 @@
> > -/*
> > - * cache.c - simple cache clean+invalidate code
> > - *
> > - * Copyright (C) 2015 ARM Limited. All rights reserved.
> > - *
> > - * Use of this source code is governed by a BSD-style license that can be
> > - * found in the LICENSE.txt file.
> > - */
> > -
> > -#include <cpu.h>
> > -
> > -void flush_caches(void)
> > -{
> > -	unsigned int level;
> > -	uint32_t clidr = read_clidr();
> > -	unsigned int max_level = (clidr >> 24) & 0x7;
> > -
> > -	uint32_t ccsidr;
> > -
> > -	if (max_level == 0)
> > -		return;
> > -
> > -	for (level = 0; level < max_level; level++) {
> > -		uint32_t cache_type = (clidr >> (level * 3)) & 0x7;
> > -		unsigned int line_size, num_ways, num_sets, way_shift;
> > -		unsigned int way, set;
> > -
> > -		if (cache_type == 1)
> > -			/* No dcache at this level */
> > -			continue;
> > -
> > -		write_csselr(level << 1);
> > -		isb();
> > -		ccsidr = read_ccsidr();
> > -
> > -		line_size = (ccsidr & 0x7) + 4; /* log2 line size */
> > -		num_ways = ((ccsidr >> 3) & 0x3ff) + 1;
> > -		num_sets = ((ccsidr >> 13) & 0x7fff) + 1;
> > -
> > -		way_shift = clz(num_ways - 1);
> > -		for (way = 0; way < num_ways; way++) {
> > -			for (set = 0; set < num_sets; set++) {
> > -				uint32_t command = level << 1;
> > -				command |= way << way_shift;
> > -				command |= set << line_size;
> > -
> > -				dccisw(command);
> > -				dsb(sy);
> > -			}
> > -		}
> > -
> > -		dsb(sy);
> > -	}
> > -	dsb(sy);
> > -	iciallu();
> > -	dsb(sy);
> > -	isb();
> > -}
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 04/12] Remove `flag_no_el3`
  2021-07-30 15:13   ` Andre Przywara
@ 2021-07-30 16:43     ` Mark Rutland
  2021-08-02 14:43       ` Mark Rutland
  0 siblings, 1 reply; 27+ messages in thread
From: Mark Rutland @ 2021-07-30 16:43 UTC (permalink / raw)
  To: Andre Przywara; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Fri, Jul 30, 2021 at 04:13:05PM +0100, Andre Przywara wrote:
> On Thu, 29 Jul 2021 16:20:42 +0100
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> Hi,
> 
> > We set `flag_no_el3` when not booted at EL3 / monitor mode, and
> > subsequently we use this to determine whether we need to drop exception
> > level before entering Linux. As this can be derived from CurrentEL or
> > CPSR, the flag itself is redundant, and we can defer the check until
> > we're about to enter Linux.
> > 
> > In future this will allow more logic to be converted into C, where it
> > will be easier to handle architectural variants.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  arch/aarch32/boot.S | 14 +++-----------
> >  arch/aarch64/boot.S | 13 ++-----------
> >  2 files changed, 5 insertions(+), 22 deletions(-)
> > 
> > diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> > index 2a85ad5..0bd1ca2 100644
> > --- a/arch/aarch32/boot.S
> > +++ b/arch/aarch32/boot.S
> > @@ -31,9 +31,6 @@ ENTRY(_start)
> >  	cmp	r0, #PSR_HYP
> >  	bne	_switch_monitor
> 
> Can't this become "beq start_no_el3" now?

I'm working to *remove* the el3/no_el3 labels, and handle the specific
exception levels as required, so I don't want to introduce that.

This says exactly what it does (i.e. switch to monitor mode), so I'd
rather leave it as-is.

> > -	mov	r0, #1
> > -	ldr	r1, =flag_no_el3
> > -	str	r0, [r1]
> >  	b	start_no_el3
> >  
> >  _switch_monitor:
> > @@ -89,9 +86,9 @@ ENTRY(jump_kernel)
> >  	ldr	lr, [r5], #4
> >  	ldm	r5, {r0 - r2}
> >  
> > -	ldr	r4, =flag_no_el3
> > -	ldr	r4, [r4]
> > -	cmp	r4, #1
> > +	mrs	r4, cpsr
> > +	and	r4, #PSR_MODE_MASK
> > +	cmp	r4, #PSR_MON
> 
> Is comparing explicitly against monitor mode the right thing? IIRC
> normally we come out of reset in secure SVC, and this *is* EL3 (the
> highest implemented exception level), from an ARMv8 perspective.

I agree it's not quite right, but the situation is more complicated:
It's more complicated than that. For details see:

* G1.4.1 "About the AArch32 PE modes"
* G1.9.1 "AArch32 state PE mode descriptions"
* G1.17 "Reset into AArch32 state" says:

The summary is:

* AArch32 doesn't necessarily reset into EL3. EL3 an EL2 are OPTIONAL.

* Supervisor mode can exist in EL3, Secure EL1, and Non-Secure EL1, and
  the PSR doesn't tell you which of the three you're in.

The boot-wrapper currently assumes we reset into EL3 or Non-Secure EL2,
and this is after the switch, where we should be in monitor mode
(otherwise PSCI cannot work, since we can't write to MVBAR). I'm not
changing that assumption. 

We should be able to rework that to *try* to switch to monitor mode, and
if that fails stick to S/NS EL1. I'm happy to tackle that as a follow
up, organising the logic so we can rely on:

* MON being EL3
* HYP being NS EL2
* SVC being S EL1 or NS EL1

> The old code did compare against HYP, which is probably what we want
> and is also one of the few modes we are sure of being not EL3.

That maches EL2 specifically (and I have left that as-is), but not about
EL3/EL1.

> 
> >  	bxeq	lr				@ no EL3
> >  
> >  	ldr	r4, =SPSR_KERNEL
> > @@ -113,8 +110,3 @@ boot_vectors:
> >  	b	.
> >  	b	.
> >  	b	.
> > -
> > -	.section .data
> > -	.align 2
> > -flag_no_el3:
> > -	.long 0
> > diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> > index 37759ce..fae0188 100644
> > --- a/arch/aarch64/boot.S
> > +++ b/arch/aarch64/boot.S
> > @@ -28,10 +28,6 @@ _start:
> >  	cmp	x0, #CURRENTEL_EL3
> >  	b.eq	1f
> 
> Can't this become "b.ne start_no_el3" now?

As above, I'm working towards removing those labels, and having a single
boot path, so I'd prefer to leave that as-is for now.

Thanks,
Mark.

> 
> Cheers,
> Andre
> 
> >  
> > -	mov	w0, #1
> > -	ldr	x1, =flag_no_el3
> > -	str	w0, [x1]
> > -
> >  	b	start_no_el3
> >  
> >  1:	mov	x0, #0x30			// RES1
> > @@ -140,8 +136,8 @@ jump_kernel:
> >  	bl	find_logical_id
> >  	bl	setup_stack		// Reset stack pointer
> >  
> > -	ldr	w0, flag_no_el3
> > -	cmp	w0, #0			// Prepare Z flag
> > +	mrs	x0, CurrentEl
> > +	cmp	w0, #CURRENTEL_EL3	// Prepare Z flag
> >  
> >  	mov	x0, x20
> >  	mov	x1, x21
> > @@ -164,8 +160,3 @@ jump_kernel:
> >  	eret
> >  
> >  	.ltorg
> > -
> > -	.data
> > -	.align 3
> > -flag_no_el3:
> > -	.long 0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 08/12] Consistently use logical CPU IDs
  2021-07-29 15:20 ` [boot-wrapper PATCH 08/12] Consistently use logical CPU IDs Mark Rutland
@ 2021-07-30 17:38   ` Andre Przywara
  0 siblings, 0 replies; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 17:38 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:46 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> In some places we assume that the cpu with MPIDR_EL1.Aff* == 0 is the
> same as logical CPU 0. While this is almost certainly true, it would be
> best to consistently use the ligical ID.

                               logical

Indeed, and I think can we can configure the model to have the primary
CPU with an MPIDR != 0.

> 
> Add a new `this_cpu_logical_id()` helper, and use this in preference to
> checking the MPIDR_EL1.Aff* bits directly.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  gic-v3.c      | 4 +---
>  gic.c         | 3 +--
>  include/cpu.h | 2 ++
>  psci.c        | 5 ++---
>  4 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/gic-v3.c b/gic-v3.c
> index ae2d2bc..62f9676 100644
> --- a/gic-v3.c
> +++ b/gic-v3.c
> @@ -101,8 +101,6 @@ void gic_secure_init_primary(void)
>  
>  void gic_secure_init(void)
>  {
> -	uint32_t cpu = read_mpidr();
> -
>  	uint32_t sre;
>  
>  	/*
> @@ -113,7 +111,7 @@ void gic_secure_init(void)
>  	if (!has_gicv3_sysreg())
>  		return;
>  
> -	if (cpu == 0)
> +	if (this_cpu_logical_id() == 0)
>  		gic_secure_init_primary();
>  
>  	sre = gic_read_icc_sre();
> diff --git a/gic.c b/gic.c
> index a84779e..04d4289 100644
> --- a/gic.c
> +++ b/gic.c
> @@ -31,14 +31,13 @@ void gic_secure_init(void)
>  {
>  	unsigned int i;
>  
> -	uint32_t cpu = read_mpidr();
>  	void *gicd_base = (void *)GIC_DIST_BASE;
>  	void *gicc_base = (void *)GIC_CPU_BASE;
>  
>  	/* Set local interrupts to Group 1 (those fields are banked) */
>  	raw_writel(~0, gicd_base + GICD_IGROUPRn);
>  
> -	if (cpu == 0) {
> +	if (this_cpu_logical_id() == 0) {
>  		uint32_t typer = raw_readl(gicd_base + GICD_TYPER);
>  
>  		/* Set SPIs to Group 1 */
> diff --git a/include/cpu.h b/include/cpu.h
> index 704d6d8..4bfb172 100644
> --- a/include/cpu.h
> +++ b/include/cpu.h
> @@ -25,5 +25,7 @@
>  
>  unsigned int find_logical_id(unsigned long mpidr);
>  
> +#define this_cpu_logical_id()	find_logical_id(read_mpidr())
> +
>  #endif /* !__ASSEMBLY__ */
>  #endif
> diff --git a/psci.c b/psci.c
> index e5d54b7..df5b0f2 100644
> --- a/psci.c
> +++ b/psci.c
> @@ -35,7 +35,7 @@ static int psci_cpu_on(unsigned long target_mpidr, unsigned long address)
>  {
>  	int ret;
>  	unsigned int cpu = find_logical_id(target_mpidr);
> -	unsigned int this_cpu = find_logical_id(read_mpidr());
> +	unsigned int this_cpu = this_cpu_logical_id();
>  
>  	if (cpu == MPIDR_INVALID)
>  		return PSCI_RET_INVALID_PARAMETERS;
> @@ -49,8 +49,7 @@ static int psci_cpu_on(unsigned long target_mpidr, unsigned long address)
>  
>  static int psci_cpu_off(void)
>  {
> -	unsigned long mpidr = read_mpidr();
> -	unsigned int cpu = find_logical_id(mpidr);
> +	unsigned int cpu = this_cpu_logical_id();
>  
>  	if (cpu == MPIDR_INVALID)
>  		return PSCI_RET_DENIED;


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 09/12] Cleanup `.globl` usage
  2021-07-29 15:20 ` [boot-wrapper PATCH 09/12] Cleanup `.globl` usage Mark Rutland
@ 2021-07-30 17:39   ` Andre Przywara
  0 siblings, 0 replies; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 17:39 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:47 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> In some places we use `ENTRY()` to mark an assembly function, whereas in
> others we just use `.globl`. Where we use `.globl` for functions or
> data, we don't keep this close to the actual definition.
> 
> Further, `ENTRY()` is a keyword in linker script with a different
> meaning, and so it would be nicer if we didn't use the same term in the
> assembly files.
> 
> This patch adds `ASM_FUNC()` and `ASM_DATA()` markers, and uses them
> consistently throughout the codebase.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Yes, replayed the transformations, they look correct.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arch/aarch32/boot.S  |  4 ++--
>  arch/aarch32/psci.S  |  4 ++--
>  arch/aarch32/stack.S | 11 +++--------
>  arch/aarch32/utils.S |  4 ++--
>  arch/aarch64/boot.S  |  8 +++-----
>  arch/aarch64/psci.S  |  9 +++------
>  arch/aarch64/spin.S  |  8 +++-----
>  arch/aarch64/stack.S | 11 ++++-------
>  arch/aarch64/utils.S |  9 +++------
>  include/linkage.h    |  6 +++++-
>  10 files changed, 30 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> index 0bd1ca2..15e51d4 100644
> --- a/arch/aarch32/boot.S
> +++ b/arch/aarch32/boot.S
> @@ -16,7 +16,7 @@
>  	.arch_extension virt
>  
>  	.section .init
> -ENTRY(_start)
> +ASM_FUNC(_start)
>  	/* Stack initialisation */
>  	cpuid	r0, r1
>  	bl	find_logical_id
> @@ -67,7 +67,7 @@ _spin_dead:
>  	 * r0: kernel address
>  	 * r1-r3, sp[0]: kernel arguments
>  	 */
> -ENTRY(jump_kernel)
> +ASM_FUNC(jump_kernel)
>  	sub	sp, #4				@ Ignore fourth argument
>  	push	{r0 - r3}
>  	mov	r5, sp
> diff --git a/arch/aarch32/psci.S b/arch/aarch32/psci.S
> index 0b7663e..dc7aeb7 100644
> --- a/arch/aarch32/psci.S
> +++ b/arch/aarch32/psci.S
> @@ -38,12 +38,12 @@ handle_smc:
>  	pop	{r4 - r12, lr}
>  	movs	pc, lr
>  
> -ENTRY(start_el3)
> +ASM_FUNC(start_el3)
>  	ldr	r0, =smc_vectors
>  	blx	setup_vector
>  	/* pass through */
>  
> -ENTRY(start_no_el3)
> +ASM_FUNC(start_no_el3)
>  	/*
>  	 * For no-el3, we assume that firmware launched the boot-wrapper in
>  	 * non-secure EL2 or EL1. We assume it has its own PSCI implementation
> diff --git a/arch/aarch32/stack.S b/arch/aarch32/stack.S
> index 59f3f52..ac6885b 100644
> --- a/arch/aarch32/stack.S
> +++ b/arch/aarch32/stack.S
> @@ -6,18 +6,13 @@
>   * Use of this source code is governed by a BSD-style license that can be
>   * found in the LICENSE.txt file.
>   */
> -
> -	.globl setup_stack
> -	.globl stack_top
> -	.globl stack_bottom
> -
>  	.text
>  	/*
>  	 * Setup initial stack pointer
>  	 * r0: logical CPU ID
>  	 * Clobbers r1 and r2
>  	 */
> -setup_stack:
> +ASM_FUNC(setup_stack)
>  	mov	r1, #STACK_SIZE
>  	ldr	r2, =stack_top
>  	mls	sp, r0, r1, r2
> @@ -25,8 +20,8 @@ setup_stack:
>  
>  	.section .stack
>  	.align 2
> -stack_bottom:
> +ASM_DATA(stack_bottom)
>  	.irp cpu, CPU_IDS
>  	.space STACK_SIZE
>  	.endr
> -stack_top:
> +ASM_DATA(stack_top)
> diff --git a/arch/aarch32/utils.S b/arch/aarch32/utils.S
> index 53d8747..5809f48 100644
> --- a/arch/aarch32/utils.S
> +++ b/arch/aarch32/utils.S
> @@ -18,7 +18,7 @@
>   * Returns MPIDR_INVALID for unknown MPIDRs
>   * Clobbers r1, r2, r3.
>   */
> -ENTRY(find_logical_id)
> +ASM_FUNC(find_logical_id)
>  	ldr	r2, =id_table
>  	mov	r1, #0
>  1:	mov	r3, #NR_CPUS
> @@ -40,7 +40,7 @@ ENTRY(find_logical_id)
>   * Setup EL3 vectors.
>   * r0: vector address
>   */
> -ENTRY(setup_vector)
> +ASM_FUNC(setup_vector)
>  	mcr	p15, 0, r0, c12, c0, 1	@ MVBAR
>  	isb
>  	bx	lr
> diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> index fae0188..ca73bcd 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -6,15 +6,13 @@
>   * Use of this source code is governed by a BSD-style license that can be
>   * found in the LICENSE.txt file.
>   */
> +#include <linkage.h>
>  
>  #include "common.S"
>  
>  	.section .init
>  
> -	.globl	_start
> -	.globl jump_kernel
> -
> -_start:
> +ASM_FUNC(_start)
>  	cpuid	x0, x1
>  	bl	find_logical_id
>  	cmp	x0, #MPIDR_INVALID
> @@ -119,7 +117,7 @@ err_invalid_id:
>  	 * x0:		entry address
>  	 * x1-x4:	arguments
>  	 */
> -jump_kernel:
> +ASM_FUNC(jump_kernel)
>  	mov	x19, x0
>  	mov	x20, x1
>  	mov	x21, x2
> diff --git a/arch/aarch64/psci.S b/arch/aarch64/psci.S
> index 6dbca11..8bd224b 100644
> --- a/arch/aarch64/psci.S
> +++ b/arch/aarch64/psci.S
> @@ -6,6 +6,7 @@
>   * Use of this source code is governed by a BSD-style license that can be
>   * found in the LICENSE.txt file.
>   */
> +#include <linkage.h>
>  #include <psci.h>
>  
>  #include "common.S"
> @@ -45,9 +46,6 @@ vector:
>  
>  	.text
>  
> -	.globl start_no_el3
> -	.globl start_el3
> -
>  err_exception:
>  	b err_exception
>  
> @@ -81,8 +79,7 @@ smc_exit:
>  	ldp	x18, x19, [sp], #16
>  	eret
>  
> -
> -start_el3:
> +ASM_FUNC(start_el3)
>  	ldr	x0, =vector
>  	bl	setup_vector
>  
> @@ -95,7 +92,7 @@ start_el3:
>   * This PSCI implementation requires EL3. Without EL3 we'll only boot the
>   * primary cpu, all others will be trapped in an infinite loop.
>   */
> -start_no_el3:
> +ASM_FUNC(start_no_el3)
>  	cpuid	x0, x1
>  	bl	find_logical_id
>  	cbz	x0, psci_first_spin
> diff --git a/arch/aarch64/spin.S b/arch/aarch64/spin.S
> index ca05937..1ea1c0b 100644
> --- a/arch/aarch64/spin.S
> +++ b/arch/aarch64/spin.S
> @@ -6,16 +6,14 @@
>   * Use of this source code is governed by a BSD-style license that can be
>   * found in the LICENSE.txt file.
>   */
> +#include <linkage.h>
>  
>  #include "common.S"
>  
>  	.text
>  
> -	.globl start_no_el3
> -	.globl start_el3
> -
> -start_el3:
> -start_no_el3:
> +ASM_FUNC(start_el3)
> +ASM_FUNC(start_no_el3)
>  	cpuid	x0, x1
>  	bl	find_logical_id
>  
> diff --git a/arch/aarch64/stack.S b/arch/aarch64/stack.S
> index 8fb38ba..c89c388 100644
> --- a/arch/aarch64/stack.S
> +++ b/arch/aarch64/stack.S
> @@ -6,10 +6,7 @@
>   * Use of this source code is governed by a BSD-style license that can be
>   * found in the LICENSE.txt file.
>   */
> -
> -	.globl setup_stack
> -	.globl stack_top
> -	.globl stack_bottom
> +#include <linkage.h>
>  
>  	.text
>  	/*
> @@ -17,7 +14,7 @@
>  	 * x0: logical CPU ID
>  	 * Clobbers x1 and x2
>  	 */
> -setup_stack:
> +ASM_FUNC(setup_stack)
>  	mov	w1, #STACK_SIZE
>  	ldr	x2, =stack_top
>  	umsubl	x0, w0, w1, x2			// sp = st_base - cpu * st_size
> @@ -26,8 +23,8 @@ setup_stack:
>  
>  	.section .stack
>  	.align 4
> -stack_bottom:
> +ASM_DATA(stack_bottom)
>  	.irp cpu, CPU_IDS
>  	.space STACK_SIZE
>  	.endr
> -stack_top:
> +ASM_DATA(stack_top)
> diff --git a/arch/aarch64/utils.S b/arch/aarch64/utils.S
> index ae22ea7..85c7f8a 100644
> --- a/arch/aarch64/utils.S
> +++ b/arch/aarch64/utils.S
> @@ -6,11 +6,8 @@
>   * Use of this source code is governed by a BSD-style license that can be
>   * found in the LICENSE.txt file.
>   */
> -
>  #include <cpu.h>
> -
> -	.globl find_logical_id
> -	.globl setup_vector
> +#include <linkage.h>
>  
>  	.text
>  
> @@ -20,7 +17,7 @@
>   * Sets the Z flag when CPU is primary
>   * Clobbers x1, x2, x3
>   */
> -find_logical_id:
> +ASM_FUNC(find_logical_id)
>  	ldr	x2, =id_table
>  	mov	x1, xzr
>  1:	mov	x3, #NR_CPUS	// check we haven't walked off the end of the array
> @@ -40,7 +37,7 @@ find_logical_id:
>   * Setup EL3 vectors
>   * x0: vector address
>   */
> -setup_vector:
> +ASM_FUNC(setup_vector)
>  	msr	VBAR_EL3, x0
>  	isb
>  	ret
> diff --git a/include/linkage.h b/include/linkage.h
> index 844a811..db8b204 100644
> --- a/include/linkage.h
> +++ b/include/linkage.h
> @@ -13,10 +13,14 @@
>  
>  #ifdef __ASSEMBLY__
>  
> -#define ENTRY(name)				\
> +#define ASM_FUNC(name)				\
>  	.globl name;				\
>  	.type  name, %function;			\
>  	name:
>  
> +#define ASM_DATA(name)				\
> +	.globl name;				\
> +	name:
> +
>  #endif /* __ASSEMBLY__ */
>  #endif


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 10/12] aarch32: rename `_spin_dead` -> `err_invalid_id`
  2021-07-29 15:20 ` [boot-wrapper PATCH 10/12] aarch32: rename `_spin_dead` -> `err_invalid_id` Mark Rutland
@ 2021-07-30 17:39   ` Andre Przywara
  0 siblings, 0 replies; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 17:39 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:48 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> For clarity, align aarch32 with aarch64, sending unexpected CPUs to an
> `err_invalid_id` loop rather than `_spin_dead`.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  arch/aarch32/boot.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> index 15e51d4..9e5c308 100644
> --- a/arch/aarch32/boot.S
> +++ b/arch/aarch32/boot.S
> @@ -21,7 +21,7 @@ ASM_FUNC(_start)
>  	cpuid	r0, r1
>  	bl	find_logical_id
>  	cmp	r0, #MPIDR_INVALID
> -	beq	_spin_dead
> +	beq	err_invalid_id
>  
>  	bl	setup_stack
>  
> @@ -58,7 +58,7 @@ _monitor:
>  	/* Initialise boot method */
>  	b	start_el3
>  
> -_spin_dead:
> +err_invalid_id:
>  	b	.
>  
>  	.text


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 11/12] Rename `spin.h` -> `boot.h`
  2021-07-29 15:20 ` [boot-wrapper PATCH 11/12] Rename `spin.h` -> `boot.h` Mark Rutland
@ 2021-07-30 17:39   ` Andre Przywara
  0 siblings, 0 replies; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 17:39 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:49 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> In `spin.h` we have function prototypes provided by `boot.c`. For
> clarity, let's align the naming.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

> ---
>  boot_common.c              | 2 +-
>  include/{spin.h => boot.h} | 6 +++---
>  psci.c                     | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
>  rename include/{spin.h => boot.h} (88%)
> 
> diff --git a/boot_common.c b/boot_common.c
> index d48b7e1..eada179 100644
> --- a/boot_common.c
> +++ b/boot_common.c
> @@ -6,8 +6,8 @@
>   * Use of this source code is governed by a BSD-style license that can be
>   * found in the LICENSE.txt file.
>   */
> +#include <boot.h>
>  #include <cpu.h>
> -#include <spin.h>
>  
>  extern unsigned long entrypoint;
>  extern unsigned long dtb;
> diff --git a/include/spin.h b/include/boot.h
> similarity index 88%
> rename from include/spin.h
> rename to include/boot.h
> index 15bbe27..d75e013 100644
> --- a/include/spin.h
> +++ b/include/boot.h
> @@ -1,13 +1,13 @@
>  /*
> - * include/spin.h
> + * include/boot.h
>   *
>   * Copyright (C) 2015 ARM Limited. All rights reserved.
>   *
>   * Use of this source code is governed by a BSD-style license that can be
>   * found in the LICENSE.txt file.
>   */
> -#ifndef __SPIN_H
> -#define __SPIN_H
> +#ifndef __BOOT_H
> +#define __BOOT_H
>  
>  #include <compiler.h>
>  
> diff --git a/psci.c b/psci.c
> index df5b0f2..a0e8700 100644
> --- a/psci.c
> +++ b/psci.c
> @@ -10,9 +10,9 @@
>  #include <stdint.h>
>  
>  #include <bakery_lock.h>
> +#include <boot.h>
>  #include <cpu.h>
>  #include <psci.h>
> -#include <spin.h>
>  
>  #ifndef CPU_IDS
>  #error "No MPIDRs provided"


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 12/12] Move common source files to `common` directory
  2021-07-29 15:20 ` [boot-wrapper PATCH 12/12] Move common source files to `common` directory Mark Rutland
@ 2021-07-30 17:40   ` Andre Przywara
  0 siblings, 0 replies; 27+ messages in thread
From: Andre Przywara @ 2021-07-30 17:40 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Thu, 29 Jul 2021 16:20:50 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

> The top-level directory is getting increasingly cluttered. For clarity
> let's move the common source files into their own directory. At the same
> time let's clean up the way we generate object lists so that it's
> consistent for arch/common objects, and doesn't require special casing
> each optional object.
> 
> Note that we also need to create a common/ directory for out-of-tree
> builds.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

Looks like a good cleanup.

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks,
Andre

> ---
>  Makefile.am                           | 29 +++++++++++++++++------------
>  bakery_lock.c => common/bakery_lock.c |  0
>  boot_common.c => common/boot.c        |  2 +-
>  gic-v3.c => common/gic-v3.c           |  0
>  gic.c => common/gic.c                 |  0
>  lib.c => common/lib.c                 |  0
>  platform.c => common/platform.c       |  0
>  psci.c => common/psci.c               |  0
>  8 files changed, 18 insertions(+), 13 deletions(-)
>  rename bakery_lock.c => common/bakery_lock.c (100%)
>  rename boot_common.c => common/boot.c (96%)
>  rename gic-v3.c => common/gic-v3.c (100%)
>  rename gic.c => common/gic.c (100%)
>  rename lib.c => common/lib.c (100%)
>  rename platform.c => common/platform.c (100%)
>  rename psci.c => common/psci.c (100%)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 5d34cc8..68f23d3 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -33,7 +33,10 @@ PSCI_CPU_ON	:= 0xc4000003
>  endif
>  PSCI_CPU_OFF	:= 0x84000002
>  
> -OFILES		=
> +COMMON_SRC	:= common/
> +COMMON_OBJ	:= boot.o bakery_lock.o platform.o lib.o
> +
> +ARCH_OBJ	:= boot.o stack.o utils.o
>  
>  if BOOTWRAPPER_32
>  CPPFLAGS	+= -DBOOTWRAPPER_32
> @@ -45,8 +48,8 @@ ARCH_SRC	:= arch/aarch64/
>  endif
>  
>  if PSCI
> -BOOTMETHOD	:= psci.o
> -OFILES		+= psci.o
> +ARCH_OBJ	+= psci.o
> +COMMON_OBJ	+= psci.o
>  PSCI_NODE	:= psci {				\
>  			compatible = \"arm,psci\";	\
>  			method = \"smc\";		\
> @@ -55,7 +58,7 @@ PSCI_NODE	:= psci {				\
>  		   };
>  CPU_NODES	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/addpsci.pl $(KERNEL_DTB))
>  else
> -BOOTMETHOD	:= spin.o
> +ARCH_OBJ	+= spin.o
>  PSCI_NODE	:=
>  CPU_NODES	:=
>  endif
> @@ -65,13 +68,13 @@ GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNE
>  GIC_RDIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,gic-v3')
>  DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
>  DEFINES		+= -DGIC_RDIST_BASE=$(GIC_RDIST_BASE)
> -GIC		:= gic-v3.o
> +COMMON_OBJ	+= gic-v3.o
>  else
>  GIC_DIST_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 0 'arm,cortex-a15-gic')
>  GIC_CPU_BASE	:= $(shell perl -I $(SCRIPT_DIR) $(SCRIPT_DIR)/findbase.pl $(KERNEL_DTB) 1 'arm,cortex-a15-gic')
>  DEFINES		+= -DGIC_CPU_BASE=$(GIC_CPU_BASE)
>  DEFINES		+= -DGIC_DIST_BASE=$(GIC_DIST_BASE)
> -GIC		:= gic.o
> +COMMON_OBJ	+= gic.o
>  endif
>  
>  if KERNEL_32
> @@ -125,8 +128,7 @@ CFLAGS		+= -Wall -fomit-frame-pointer
>  CFLAGS		+= -ffunction-sections -fdata-sections
>  LDFLAGS		+= --gc-sections
>  
> -OFILES		+= boot_common.o bakery_lock.o platform.o $(GIC) lib.o
> -OFILES		+= $(addprefix $(ARCH_SRC),boot.o stack.o $(BOOTMETHOD) utils.o)
> +OBJ		:= $(addprefix $(ARCH_SRC),$(ARCH_OBJ)) $(addprefix $(COMMON_SRC),$(COMMON_OBJ))
>  
>  # Don't lookup all prerequisites in $(top_srcdir), only the source files. When
>  # building outside the source tree $(ARCH_SRC) needs to be created.
> @@ -136,18 +138,21 @@ vpath %.S $(top_srcdir)
>  
>  all: $(IMAGE)
>  
> -CLEANFILES = $(IMAGE) linux-system.axf xen-system.axf $(OFILES) model.lds fdt.dtb
> +CLEANFILES = $(IMAGE) linux-system.axf xen-system.axf $(OBJ) model.lds fdt.dtb
>  
> -$(IMAGE): $(OFILES) model.lds fdt.dtb $(KERNEL_IMAGE) $(FILESYSTEM) $(XEN_IMAGE)
> -	$(LD) $(LDFLAGS) $(OFILES) -o $@ --script=model.lds
> +$(IMAGE): $(OBJ) model.lds fdt.dtb $(KERNEL_IMAGE) $(FILESYSTEM) $(XEN_IMAGE)
> +	$(LD) $(LDFLAGS) $(OBJ) -o $@ --script=model.lds
>  
>  $(ARCH_SRC):
>  	$(MKDIR_P) $@
>  
> +$(COMMON_SRC):
> +	$(MKDIR_P) $@
> +
>  %.o: %.S Makefile | $(ARCH_SRC)
>  	$(CC) $(CPPFLAGS) -D__ASSEMBLY__ $(CFLAGS) $(DEFINES) -c -o $@ $<
>  
> -%.o: %.c Makefile
> +%.o: %.c Makefile | $(COMMON_SRC)
>  	$(CC) $(CPPFLAGS) $(CFLAGS) $(DEFINES) -c -o $@ $<
>  
>  model.lds: $(LD_SCRIPT) Makefile
> diff --git a/bakery_lock.c b/common/bakery_lock.c
> similarity index 100%
> rename from bakery_lock.c
> rename to common/bakery_lock.c
> diff --git a/boot_common.c b/common/boot.c
> similarity index 96%
> rename from boot_common.c
> rename to common/boot.c
> index eada179..c74d34c 100644
> --- a/boot_common.c
> +++ b/common/boot.c
> @@ -1,5 +1,5 @@
>  /*
> - * boot_common.c - common spin function for all boot methods
> + * boot.c - common spin function for all boot methods
>   *
>   * Copyright (C) 2015 ARM Limited. All rights reserved.
>   *
> diff --git a/gic-v3.c b/common/gic-v3.c
> similarity index 100%
> rename from gic-v3.c
> rename to common/gic-v3.c
> diff --git a/gic.c b/common/gic.c
> similarity index 100%
> rename from gic.c
> rename to common/gic.c
> diff --git a/lib.c b/common/lib.c
> similarity index 100%
> rename from lib.c
> rename to common/lib.c
> diff --git a/platform.c b/common/platform.c
> similarity index 100%
> rename from platform.c
> rename to common/platform.c
> diff --git a/psci.c b/common/psci.c
> similarity index 100%
> rename from psci.c
> rename to common/psci.c


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [boot-wrapper PATCH 04/12] Remove `flag_no_el3`
  2021-07-30 16:43     ` Mark Rutland
@ 2021-08-02 14:43       ` Mark Rutland
  0 siblings, 0 replies; 27+ messages in thread
From: Mark Rutland @ 2021-08-02 14:43 UTC (permalink / raw)
  To: Andre Przywara; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Fri, Jul 30, 2021 at 05:43:33PM +0100, Mark Rutland wrote:
> On Fri, Jul 30, 2021 at 04:13:05PM +0100, Andre Przywara wrote:
> > On Thu, 29 Jul 2021 16:20:42 +0100
> > Mark Rutland <mark.rutland@arm.com> wrote:
> > 
> > Hi,
> > 
> > > We set `flag_no_el3` when not booted at EL3 / monitor mode, and
> > > subsequently we use this to determine whether we need to drop exception
> > > level before entering Linux. As this can be derived from CurrentEL or
> > > CPSR, the flag itself is redundant, and we can defer the check until
> > > we're about to enter Linux.
> > > 
> > > In future this will allow more logic to be converted into C, where it
> > > will be easier to handle architectural variants.
> > > 
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > ---
> > >  arch/aarch32/boot.S | 14 +++-----------
> > >  arch/aarch64/boot.S | 13 ++-----------
> > >  2 files changed, 5 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> > > index 2a85ad5..0bd1ca2 100644
> > > --- a/arch/aarch32/boot.S
> > > +++ b/arch/aarch32/boot.S
> > > @@ -31,9 +31,6 @@ ENTRY(_start)
> > >  	cmp	r0, #PSR_HYP
> > >  	bne	_switch_monitor
> > 
> > Can't this become "beq start_no_el3" now?
> 
> I'm working to *remove* the el3/no_el3 labels, and handle the specific
> exception levels as required, so I don't want to introduce that.
> 
> This says exactly what it does (i.e. switch to monitor mode), so I'd
> rather leave it as-is.
> 
> > > -	mov	r0, #1
> > > -	ldr	r1, =flag_no_el3
> > > -	str	r0, [r1]
> > >  	b	start_no_el3
> > >  
> > >  _switch_monitor:
> > > @@ -89,9 +86,9 @@ ENTRY(jump_kernel)
> > >  	ldr	lr, [r5], #4
> > >  	ldm	r5, {r0 - r2}
> > >  
> > > -	ldr	r4, =flag_no_el3
> > > -	ldr	r4, [r4]
> > > -	cmp	r4, #1
> > > +	mrs	r4, cpsr
> > > +	and	r4, #PSR_MODE_MASK
> > > +	cmp	r4, #PSR_MON
> > 
> > Is comparing explicitly against monitor mode the right thing? IIRC
> > normally we come out of reset in secure SVC, and this *is* EL3 (the
> > highest implemented exception level), from an ARMv8 perspective.
> 
> I agree it's not quite right, but the situation is more complicated:
> It's more complicated than that. For details see:
> 
> * G1.4.1 "About the AArch32 PE modes"
> * G1.9.1 "AArch32 state PE mode descriptions"
> * G1.17 "Reset into AArch32 state" says:
> 
> The summary is:
> 
> * AArch32 doesn't necessarily reset into EL3. EL3 an EL2 are OPTIONAL.
> 
> * Supervisor mode can exist in EL3, Secure EL1, and Non-Secure EL1, and
>   the PSR doesn't tell you which of the three you're in.
> 
> The boot-wrapper currently assumes we reset into EL3 or Non-Secure EL2,
> and this is after the switch, where we should be in monitor mode
> (otherwise PSCI cannot work, since we can't write to MVBAR). I'm not
> changing that assumption. 

Upon reflection, I'm going to drop this patch from the series for now
and rework it to make the above clearer and more robust...

> We should be able to rework that to *try* to switch to monitor mode, and
> if that fails stick to S/NS EL1. I'm happy to tackle that as a follow
> up, organising the logic so we can rely on:
> 
> * MON being EL3
> * HYP being NS EL2
> * SVC being S EL1 or NS EL1

... and to try to make this true as a first step.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2021-08-02 14:45 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 15:20 [boot-wrapper PATCH 00/12] Preparatory fixes and cleanup Mark Rutland
2021-07-29 15:20 ` [boot-wrapper PATCH 01/12] Ensure `kernel_address` is aligned Mark Rutland
2021-07-30 15:11   ` Andre Przywara
2021-07-29 15:20 ` [boot-wrapper PATCH 02/12] Output text separately from data Mark Rutland
2021-07-29 15:20 ` [boot-wrapper PATCH 03/12] Remove cache maintenance Mark Rutland
2021-07-30 15:12   ` Andre Przywara
2021-07-30 15:43     ` Mark Rutland
2021-07-29 15:20 ` [boot-wrapper PATCH 04/12] Remove `flag_no_el3` Mark Rutland
2021-07-30 15:13   ` Andre Przywara
2021-07-30 16:43     ` Mark Rutland
2021-08-02 14:43       ` Mark Rutland
2021-07-29 15:20 ` [boot-wrapper PATCH 05/12] Move PSCI triage to C Mark Rutland
2021-07-29 15:20 ` [boot-wrapper PATCH 06/12] Move scripts to a `scripts` directory Mark Rutland
2021-07-30 15:13   ` Andre Przywara
2021-07-29 15:20 ` [boot-wrapper PATCH 07/12] aarch64: respect text offset Mark Rutland
2021-07-30 15:13   ` Andre Przywara
2021-07-30 15:43     ` Mark Rutland
2021-07-29 15:20 ` [boot-wrapper PATCH 08/12] Consistently use logical CPU IDs Mark Rutland
2021-07-30 17:38   ` Andre Przywara
2021-07-29 15:20 ` [boot-wrapper PATCH 09/12] Cleanup `.globl` usage Mark Rutland
2021-07-30 17:39   ` Andre Przywara
2021-07-29 15:20 ` [boot-wrapper PATCH 10/12] aarch32: rename `_spin_dead` -> `err_invalid_id` Mark Rutland
2021-07-30 17:39   ` Andre Przywara
2021-07-29 15:20 ` [boot-wrapper PATCH 11/12] Rename `spin.h` -> `boot.h` Mark Rutland
2021-07-30 17:39   ` Andre Przywara
2021-07-29 15:20 ` [boot-wrapper PATCH 12/12] Move common source files to `common` directory Mark Rutland
2021-07-30 17:40   ` Andre Przywara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).