* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.