From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:23:39
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
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:24:45
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(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:26:01
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(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:27:30
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
@@ -123,7 +123,7 @@ CFLAGS += -Wall -fomit-frame-pointerCFLAGS+=-ffunction-sections-fdata-sectionsLDFLAGS+=--gc-sections-OFILES+=boot_common.obakery_lock.oplatform.o$(GIC)cache.olib.o+OFILES+=boot_common.obakery_lock.oplatform.o$(GIC)lib.oOFILES+=$(addprefix$(ARCH_SRC),boot.ostack.o$(BOOTMETHOD)utils.o)# Don't lookup all prerequisites in $(top_srcdir), only the source files. When
@@ -13,7 +13,6 @@ extern unsigned long entrypoint;externunsignedlongdtb;voidinit_platform(void);-voidflush_caches(void);void__noreturnjump_kernel(unsignedlongaddress,unsignedlonga0,
@@ -62,8 +61,6 @@ void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry)void__noreturnfirst_spin(unsignedintcpu,unsignedlong*mbox,unsignedlonginvalid){-flush_caches();-if(cpu==0){init_platform();
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:28:27
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(-)
@@ -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:=$(shellperl-I$(top_srcdir)$(top_srcdir)/findmem.pl$(KERNEL_DTB))-UART_BASE:=$(shellperl-I$(top_srcdir)$(top_srcdir)/findbase.pl$(KERNEL_DTB)0'arm,pl011')-SYSREGS_BASE:=$(shellperl-I$(top_srcdir)$(top_srcdir)/findbase.pl$(KERNEL_DTB)0'arm,vexpress-sysreg'2>/dev/null)+PHYS_OFFSET:=$(shellperl-I$(SCRIPT_DIR)$(SCRIPT_DIR)/findmem.pl$(KERNEL_DTB))+UART_BASE:=$(shellperl-I$(SCRIPT_DIR)$(SCRIPT_DIR)/findbase.pl$(KERNEL_DTB)0'arm,pl011')+SYSREGS_BASE:=$(shellperl-I$(SCRIPT_DIR)$(SCRIPT_DIR)/findbase.pl$(KERNEL_DTB)0'arm,vexpress-sysreg'2>/dev/null)CNTFRQ:=0x01800000# 24Mhz-CPU_IDS:=$(shellperl-I$(top_srcdir)$(top_srcdir)/findcpuids.pl$(KERNEL_DTB))+CPU_IDS:=$(shellperl-I$(SCRIPT_DIR)$(SCRIPT_DIR)/findcpuids.pl$(KERNEL_DTB))NR_CPUS:=$(shellecho$(CPU_IDS)|tr','' '|wc-w)DEFINES=-DCNTFRQ=$(CNTFRQ)
diff --git a/FDT.pm b/scripts/FDT.pmsimilarity index 100%rename from FDT.pmrename to scripts/FDT.pmdiff --git a/addpsci.pl b/scripts/addpsci.plsimilarity index 100%rename from addpsci.plrename to scripts/addpsci.pldiff --git a/findbase.pl b/scripts/findbase.plsimilarity index 100%rename from findbase.plrename to scripts/findbase.pldiff --git a/findcpuids.pl b/scripts/findcpuids.plsimilarity index 100%rename from findcpuids.plrename to scripts/findcpuids.pldiff --git a/findmem.pl b/scripts/findmem.plsimilarity index 100%rename from findmem.plrename 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
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:30:55
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(-)
@@ -31,14 +31,13 @@ void gic_secure_init(void){unsignedinti;-uint32_tcpu=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_ttyper=raw_readl(gicd_base+GICD_TYPER);/* Set SPIs to Group 1 */
@@ -35,7 +35,7 @@ static int psci_cpu_on(unsigned long target_mpidr, unsigned long address){intret;unsignedintcpu=find_logical_id(target_mpidr);-unsignedintthis_cpu=find_logical_id(read_mpidr());+unsignedintthis_cpu=this_cpu_logical_id();if(cpu==MPIDR_INVALID)returnPSCI_RET_INVALID_PARAMETERS;
@@ -49,8 +49,7 @@ static int psci_cpu_on(unsigned long target_mpidr, unsigned long address)staticintpsci_cpu_off(void){-unsignedlongmpidr=read_mpidr();-unsignedintcpu=find_logical_id(mpidr);+unsignedintcpu=this_cpu_logical_id();if(cpu==MPIDR_INVALID)returnPSCI_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
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:32:16
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(-)
@@ -31,7 +31,7 @@ static int psci_store_address(unsigned int cpu, unsigned long address)returnPSCI_RET_SUCCESS;}-intpsci_cpu_on(unsignedlongtarget_mpidr,unsignedlongaddress)+staticintpsci_cpu_on(unsignedlongtarget_mpidr,unsignedlongaddress){intret;unsignedintcpu=find_logical_id(target_mpidr);
@@ -47,7 +47,7 @@ int psci_cpu_on(unsigned long target_mpidr, unsigned long address)returnret;}-intpsci_cpu_off(void)+staticintpsci_cpu_off(void){unsignedlongmpidr=read_mpidr();unsignedintcpu=find_logical_id(mpidr);
@@ -62,6 +62,23 @@ int psci_cpu_off(void)unreachable();}+longpsci_call(unsignedlongfid,unsignedlongarg1,unsignedlongarg2)+{+switch(fid){+casePSCI_CPU_OFF:+returnpsci_cpu_off();+#ifdef KERNEL_32+casePSCI_CPU_ON_32:+returnpsci_cpu_on(arg1,arg2);+#else+casePSCI_CPU_ON_64:+returnpsci_cpu_on(arg1,arg2);+#endif+default:+returnPSCI_RET_NOT_SUPPORTED;+}+}+void__noreturnpsci_first_spin(unsignedintcpu){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
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:33:27
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(-)
@@ -20,7 +17,7 @@*SetstheZflagwhenCPUisprimary*Clobbersx1,x2,x3*/-find_logical_id:+ASM_FUNC(find_logical_id)ldrx2,=id_tablemovx1,xzr1:movx3,#NR_CPUS // check we haven't walked off the end of the array
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:34:26
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
@@ -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.++usewarnings;+usestrict;+useinteger;++packageAA64Image;++# Header definitions from v5.13+# See https://www.kernel.org/doc/html/v5.13/arm64/booting.html#call-the-kernel-image++useconstant{+HEADER_LEN=>64,+HEADER_MAGIC=>0x644d5241,+};++subparse+{+my$class=shift;+my$fh=shift;+my$self=bless{},$class;++read($fh,my$raw_header,AA64Image::HEADER_LEN)==AA64Image::HEADER_LENorgotofailed;++(+$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";+gotofailed;+}++return$self;++failed:+warn"Unable to parse header";+returnundef;+}++subget_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){+return0x80000;+}++return$self->{text_offset};+}++subget_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;
@@ -0,0 +1,28 @@+#!/usr/bin/perl-w+#FindtheloadaddressofanAArch64LinuxImage+#+#Usage:./$0<Image><min-offset>+#+#Copyright(C)2021ARMLimited.Allrightsreserved.+#+#UseofthissourcecodeisgovernedbyaBSD-stylelicensethatcanbe+#foundintheLICENSE.txtfile.++usewarnings;+usestrict;++useAA64Image;++my$filename=shift;+die("No filename provided")unlessdefined($filename);++my$min=shift;+$min=oct($min)if$min=~/^0/;++open(my$fh,"<:raw",$filename)ordie("Unable to open file '$filename'");++my$image=AA64Image->parse($fh)ordie("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
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:36:09
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(-)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:37:20
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/include/spin.h b/include/boot.hsimilarity index 88%rename from include/spin.hrename to include/boot.hindex 15bbe27..d75e013 100644--- a/include/spin.h+++ b/include/boot.h
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-29 15:38:55
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%)
@@ -125,8 +128,7 @@ CFLAGS += -Wall -fomit-frame-pointerCFLAGS+=-ffunction-sections-fdata-sectionsLDFLAGS+=--gc-sections-OFILES+=boot_common.obakery_lock.oplatform.o$(GIC)lib.o-OFILES+=$(addprefix$(ARCH_SRC),boot.ostack.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.
diff --git a/bakery_lock.c b/common/bakery_lock.csimilarity index 100%rename from bakery_lock.crename to common/bakery_lock.cdiff --git a/boot_common.c b/common/boot.csimilarity index 96%rename from boot_common.crename to common/boot.cindex eada179..c74d34c 100644--- a/boot_common.c+++ b/common/boot.c
diff --git a/gic-v3.c b/common/gic-v3.csimilarity index 100%rename from gic-v3.crename to common/gic-v3.cdiff --git a/gic.c b/common/gic.csimilarity index 100%rename from gic.crename to common/gic.cdiff --git a/lib.c b/common/lib.csimilarity index 100%rename from lib.crename to common/lib.cdiff --git a/platform.c b/common/platform.csimilarity index 100%rename from platform.crename to common/platform.cdiff --git a/psci.c b/common/psci.csimilarity index 100%rename from psci.crename 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
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 15:15:05
On Thu, 29 Jul 2021 16:20:39 +0100
Mark Rutland [off-list ref] 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
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 15:18:35
On Thu, 29 Jul 2021 16:20:41 +0100
Mark Rutland [off-list ref] 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
@@ -123,7 +123,7 @@ CFLAGS += -Wall -fomit-frame-pointerCFLAGS+=-ffunction-sections-fdata-sectionsLDFLAGS+=--gc-sections-OFILES+=boot_common.obakery_lock.oplatform.o$(GIC)cache.olib.o+OFILES+=boot_common.obakery_lock.oplatform.o$(GIC)lib.oOFILES+=$(addprefix$(ARCH_SRC),boot.ostack.o$(BOOTMETHOD)utils.o)# Don't lookup all prerequisites in $(top_srcdir), only the source files. When
@@ -13,7 +13,6 @@ extern unsigned long entrypoint;externunsignedlongdtb;voidinit_platform(void);-voidflush_caches(void);void__noreturnjump_kernel(unsignedlongaddress,unsignedlonga0,
@@ -62,8 +61,6 @@ void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry)void__noreturnfirst_spin(unsignedintcpu,unsignedlong*mbox,unsignedlonginvalid){-flush_caches();-if(cpu==0){init_platform();
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 15:19:09
On Thu, 29 Jul 2021 16:20:42 +0100
Mark Rutland [off-list ref] wrote:
Hi,
quoted hunk
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(-)
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.
quoted hunk
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
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 15:20:22
On Thu, 29 Jul 2021 16:20:44 +0100
Mark Rutland [off-list ref] 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
@@ -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:=$(shellperl-I$(top_srcdir)$(top_srcdir)/findmem.pl$(KERNEL_DTB))-UART_BASE:=$(shellperl-I$(top_srcdir)$(top_srcdir)/findbase.pl$(KERNEL_DTB)0'arm,pl011')-SYSREGS_BASE:=$(shellperl-I$(top_srcdir)$(top_srcdir)/findbase.pl$(KERNEL_DTB)0'arm,vexpress-sysreg'2>/dev/null)+PHYS_OFFSET:=$(shellperl-I$(SCRIPT_DIR)$(SCRIPT_DIR)/findmem.pl$(KERNEL_DTB))+UART_BASE:=$(shellperl-I$(SCRIPT_DIR)$(SCRIPT_DIR)/findbase.pl$(KERNEL_DTB)0'arm,pl011')+SYSREGS_BASE:=$(shellperl-I$(SCRIPT_DIR)$(SCRIPT_DIR)/findbase.pl$(KERNEL_DTB)0'arm,vexpress-sysreg'2>/dev/null)CNTFRQ:=0x01800000# 24Mhz-CPU_IDS:=$(shellperl-I$(top_srcdir)$(top_srcdir)/findcpuids.pl$(KERNEL_DTB))+CPU_IDS:=$(shellperl-I$(SCRIPT_DIR)$(SCRIPT_DIR)/findcpuids.pl$(KERNEL_DTB))NR_CPUS:=$(shellecho$(CPU_IDS)|tr','' '|wc-w)DEFINES=-DCNTFRQ=$(CNTFRQ)
diff --git a/FDT.pm b/scripts/FDT.pmsimilarity index 100%rename from FDT.pmrename to scripts/FDT.pmdiff --git a/addpsci.pl b/scripts/addpsci.plsimilarity index 100%rename from addpsci.plrename to scripts/addpsci.pldiff --git a/findbase.pl b/scripts/findbase.plsimilarity index 100%rename from findbase.plrename to scripts/findbase.pldiff --git a/findcpuids.pl b/scripts/findcpuids.plsimilarity index 100%rename from findcpuids.plrename to scripts/findcpuids.pldiff --git a/findmem.pl b/scripts/findmem.plsimilarity index 100%rename from findmem.plrename to scripts/findmem.pl
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 15:21:05
On Thu, 29 Jul 2021 16:20:45 +0100
Mark Rutland [off-list ref] wrote:
quoted hunk
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
@@ -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.++usewarnings;+usestrict;+useinteger;++packageAA64Image;++# Header definitions from v5.13+# See https://www.kernel.org/doc/html/v5.13/arm64/booting.html#call-the-kernel-image++useconstant{+HEADER_LEN=>64,+HEADER_MAGIC=>0x644d5241,+};++subparse+{+my$class=shift;+my$fh=shift;+my$self=bless{},$class;++read($fh,my$raw_header,AA64Image::HEADER_LEN)==AA64Image::HEADER_LENorgotofailed;++(+$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";+gotofailed;+}++return$self;++failed:+warn"Unable to parse header";+returnundef;+}++subget_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){+return0x80000;+}++return$self->{text_offset};+}++subget_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
quoted hunk
+ 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;
@@ -0,0 +1,28 @@+#!/usr/bin/perl-w+#FindtheloadaddressofanAArch64LinuxImage+#+#Usage:./$0<Image><min-offset>+#+#Copyright(C)2021ARMLimited.Allrightsreserved.+#+#UseofthissourcecodeisgovernedbyaBSD-stylelicensethatcanbe+#foundintheLICENSE.txtfile.++usewarnings;+usestrict;++useAA64Image;++my$filename=shift;+die("No filename provided")unlessdefined($filename);++my$min=shift;+$min=oct($min)if$min=~/^0/;++open(my$fh,"<:raw",$filename)ordie("Unable to open file '$filename'");++my$image=AA64Image->parse($fh)ordie("Unable to parse Image");++my$offset=$image->get_load_offset($min);++printf("0x%016x\n",$offset);
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-30 15:45:25
On Fri, Jul 30, 2021 at 04:13:38PM +0100, Andre Przywara wrote:
On Thu, 29 Jul 2021 16:20:45 +0100
Mark Rutland [off-list ref] wrote:
quoted
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
quoted
+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.
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-30 15:45:54
On Fri, Jul 30, 2021 at 04:12:23PM +0100, Andre Przywara wrote:
On Thu, 29 Jul 2021 16:20:41 +0100
Mark Rutland [off-list ref] wrote:
quoted
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.
quoted
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
@@ -123,7 +123,7 @@ CFLAGS += -Wall -fomit-frame-pointerCFLAGS+=-ffunction-sections-fdata-sectionsLDFLAGS+=--gc-sections-OFILES+=boot_common.obakery_lock.oplatform.o$(GIC)cache.olib.o+OFILES+=boot_common.obakery_lock.oplatform.o$(GIC)lib.oOFILES+=$(addprefix$(ARCH_SRC),boot.ostack.o$(BOOTMETHOD)utils.o)# Don't lookup all prerequisites in $(top_srcdir), only the source files. When
@@ -13,7 +13,6 @@ extern unsigned long entrypoint;externunsignedlongdtb;voidinit_platform(void);-voidflush_caches(void);void__noreturnjump_kernel(unsignedlongaddress,unsignedlonga0,
@@ -62,8 +61,6 @@ void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry)void__noreturnfirst_spin(unsignedintcpu,unsignedlong*mbox,unsignedlonginvalid){-flush_caches();-if(cpu==0){init_platform();
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-07-30 16:45:42
On Fri, Jul 30, 2021 at 04:13:05PM +0100, Andre Przywara wrote:
On Thu, 29 Jul 2021 16:20:42 +0100
Mark Rutland [off-list ref] wrote:
Hi,
quoted
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(-)
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.
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.
quoted
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
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 17:41:42
On Thu, 29 Jul 2021 16:20:46 +0100
Mark Rutland [off-list ref] 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
@@ -31,14 +31,13 @@ void gic_secure_init(void){unsignedinti;-uint32_tcpu=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_ttyper=raw_readl(gicd_base+GICD_TYPER);/* Set SPIs to Group 1 */
@@ -35,7 +35,7 @@ static int psci_cpu_on(unsigned long target_mpidr, unsigned long address){intret;unsignedintcpu=find_logical_id(target_mpidr);-unsignedintthis_cpu=find_logical_id(read_mpidr());+unsignedintthis_cpu=this_cpu_logical_id();if(cpu==MPIDR_INVALID)returnPSCI_RET_INVALID_PARAMETERS;
@@ -49,8 +49,7 @@ static int psci_cpu_on(unsigned long target_mpidr, unsigned long address)staticintpsci_cpu_off(void){-unsignedlongmpidr=read_mpidr();-unsignedintcpu=find_logical_id(mpidr);+unsignedintcpu=this_cpu_logical_id();if(cpu==MPIDR_INVALID)returnPSCI_RET_DENIED;
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 17:41:56
On Thu, 29 Jul 2021 16:20:48 +0100
Mark Rutland [off-list ref] 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
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 17:42:06
On Thu, 29 Jul 2021 16:20:47 +0100
Mark Rutland [off-list ref] 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
@@ -20,7 +17,7 @@*SetstheZflagwhenCPUisprimary*Clobbersx1,x2,x3*/-find_logical_id:+ASM_FUNC(find_logical_id)ldrx2,=id_tablemovx1,xzr1:movx3,#NR_CPUS // check we haven't walked off the end of the array
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 17:42:27
On Thu, 29 Jul 2021 16:20:49 +0100
Mark Rutland [off-list ref] 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
diff --git a/include/spin.h b/include/boot.hsimilarity index 88%rename from include/spin.hrename to include/boot.hindex 15bbe27..d75e013 100644--- a/include/spin.h+++ b/include/boot.h
From: Andre Przywara <andre.przywara@arm.com> Date: 2021-07-30 17:43:09
On Thu, 29 Jul 2021 16:20:50 +0100
Mark Rutland [off-list ref] 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
@@ -125,8 +128,7 @@ CFLAGS += -Wall -fomit-frame-pointerCFLAGS+=-ffunction-sections-fdata-sectionsLDFLAGS+=--gc-sections-OFILES+=boot_common.obakery_lock.oplatform.o$(GIC)lib.o-OFILES+=$(addprefix$(ARCH_SRC),boot.ostack.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.
diff --git a/bakery_lock.c b/common/bakery_lock.csimilarity index 100%rename from bakery_lock.crename to common/bakery_lock.cdiff --git a/boot_common.c b/common/boot.csimilarity index 96%rename from boot_common.crename to common/boot.cindex eada179..c74d34c 100644--- a/boot_common.c+++ b/common/boot.c
diff --git a/gic-v3.c b/common/gic-v3.csimilarity index 100%rename from gic-v3.crename to common/gic-v3.cdiff --git a/gic.c b/common/gic.csimilarity index 100%rename from gic.crename to common/gic.cdiff --git a/lib.c b/common/lib.csimilarity index 100%rename from lib.crename to common/lib.cdiff --git a/platform.c b/common/platform.csimilarity index 100%rename from platform.crename to common/platform.cdiff --git a/psci.c b/common/psci.csimilarity index 100%rename from psci.crename to common/psci.c
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-08-02 14:45:39
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:
quoted
On Thu, 29 Jul 2021 16:20:42 +0100
Mark Rutland [off-list ref] wrote:
Hi,
quoted
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(-)
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.
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