All of lore.kernel.org
 help / color / mirror / Atom feed
* [bootwrapper PATCH 00/13] Cleanups and improvements
@ 2022-01-11 13:06 Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 01/13] Document entry requirements Mark Rutland
                   ` (12 more replies)
  0 siblings, 13 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

This series reworks the aarch64 boot-wrapper, moving a fair amount of
initialization code to C. This has a few benefits:

1) This makes the code more legible and easier to maintain.

    Current feature detection and system register configuration is
    written in assembly, requiring runs of *almost* identical blocks of
    assembly to read ID registers and conditionally initialize register
    values. This requires a bunch of labels (which are all named
    numerically), and all the magic numbers are hard coded, so this gets
    pretty painful to read:
    
    |	mrs	x1, id_aa64isar0_el1
    |	ubfx	x1, x1, #24, #4
    |	cbz	x1, 1f
    |
    |	orr	x0, x0, #(1 << 34)		// TME enable
    |
    | 1:
    
    In C, it's much easier to add helpers which use mnemonics, which
    makes the code much easier to read, and avoids the need to manually
    allocate registers, etc:
    
    |	if (mrs_field(ID_AA64ISAR0_EL1, TME))
    |		scr |= SCR_EL3_TME;

    This should make it easier to handle new architectural extensions
    (and/or architecture variants such as ARMv8-R) in future.
    
2) This allows for better diagnostics.

    Currently a mis-configured boot-wrapper is rather unforgiving, and
    provides no indication as to what might have gone wrong. By moving
    initialization to C, we can make use to the UART code to log
    diagnostic information, and we can more easily add additional error
    handling and conditional logic.
    
    This series adds diagnostic information and error handling that can
    be used to identify problems such as the boot-wrapper being launched
    at the wrong exception level:
    
    | Boot-wrapper v0.2
    | Entered at EL2
    | Memory layout:
    | [0000000080000000..0000000080001f90] => boot-wrapper
    | [000000008000fff8..0000000080010000] => mbox
    | [0000000080200000..00000000822af200] => kernel
    | [0000000088000000..0000000088002857] => dtb
    |
    | WARNING: PSCI could not be initialized. Boot may fail

Originally I had planned for this to be a more expansive set of changes,
unifying some early control-flow, fixing some latent bugs, and making
the boot-wrapper dynamically handle being entered at any of EL{3,2,1}
with automated selection of a suitable DT. As the series has already
become pretty long, I'd like to get this preparatory cleanup out of the
way first, and handle those cases with subsequent patches.

If there are no objections in the next few days, I intend to apply these
patches by the end of the week. If people can provide Reviewed-by and
Tested-by tags, I'd be happy to apply this earlier.

I've pushed the series to the `cleanup` branch in the boot-wrapper repo:

  https://git.kernel.org/pub/scm/linux/kernel/git/mark/boot-wrapper-aarch64.git/
  git://git.kernel.org/pub/scm/linux/kernel/git/mark/boot-wrapper-aarch64.git

... and it should apply cleanup atop the `master` branch.

Thanks,
Mark.

Mark Rutland (13):
  Document entry requirements
  Add bit-field macros
  aarch64: add system register accessors
  aarch32: add coprocessor accessors
  aarch64: add mov_64 macro
  aarch64: initialize SCTLR_ELx for the boot-wrapper
  Rework common init C code
  Announce boot-wrapper mode / exception level
  aarch64: move the bulk of EL3 initialization to C
  aarch32: move the bulk of Secure PL1 initialization to C
  Announce locations of memory objects
  Rework bootmethod initialization
  Unify start_el3 & start_no_el3

 Makefile.am                       |   6 +-
 arch/aarch32/boot.S               |  33 +++---
 arch/aarch32/include/asm/cpu.h    |  62 ++++++++---
 arch/aarch32/include/asm/gic-v3.h |   6 +-
 arch/aarch32/include/asm/psci.h   |  28 +++++
 arch/aarch32/init.c               |  42 ++++++++
 arch/aarch32/psci.S               |  13 +--
 arch/aarch32/utils.S              |   9 --
 arch/aarch64/boot.S               | 169 +++++++++++++-----------------
 arch/aarch64/common.S             |  10 +-
 arch/aarch64/include/asm/cpu.h    | 102 +++++++++++++++---
 arch/aarch64/include/asm/gic-v3.h |  10 +-
 arch/aarch64/include/asm/psci.h   |  28 +++++
 arch/aarch64/init.c               |  88 ++++++++++++++++
 arch/aarch64/psci.S               |  22 +---
 arch/aarch64/spin.S               |   3 +
 arch/aarch64/utils.S              |   9 --
 common/boot.c                     |   4 -
 common/init.c                     |  60 +++++++++++
 common/platform.c                 |  46 +++++---
 common/psci.c                     |  22 +++-
 include/bits.h                    |  33 ++++++
 include/boot.h                    |   2 +
 include/platform.h                |  20 ++++
 model.lds.S                       |  20 ++--
 25 files changed, 621 insertions(+), 226 deletions(-)
 create mode 100644 arch/aarch32/include/asm/psci.h
 create mode 100644 arch/aarch32/init.c
 create mode 100644 arch/aarch64/include/asm/psci.h
 create mode 100644 arch/aarch64/init.c
 create mode 100644 common/init.c
 create mode 100644 include/bits.h
 create mode 100644 include/platform.h

-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 01/13] Document entry requirements
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 02/13] Add bit-field macros Mark Rutland
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

Currently the boot-wrapper only supports some combinations of exception
levels, with other combinations not being supported.

While we generally expect the boot-wrapper to be entered at the highest
implemented exception level, the AArch32 boot-wrapper has a comment
implying it supports being entered with something else owning EL3. As
this would require such EL3 firmware to always be in sync with the
boot-wrapper's requirements, which change over time, we don't actually
support such a configuration.

Some CPU state (such as CNTFRQ/CNTFRQ_EL0) needs to be initialized at
the highest implemented exception level, but today the boot-wrapper only
does so when entered at EL3 / Secure-PL1. Thus, today the only
completely supported configurations are EL3 / Secure-PL1, and entering
in other configurations is not entirely supported.

The aarch64 `jump_kernel` function always writes to SCTLR_EL2, which is
UNDEFINED at EL1. Hence, the aarch64 boot-wrapper does not support being
entered at EL1.

The aarch32 code assumes that any non-hyp mode is Secure PL1, and
attempt to switch to monitor mode. If entered on a system without the
security extensions, where the highest privileged mode is Non-secure
PL1, this will not work. Hence the aarch32 boot-wrapper does not support
being entered at Non-secure PL1.

Actually supporting all of these configurations requires restructuring
much of the boot-wrapper. For now, document the supported configurations
in each architecture's boot.S, and remove the misleading comment from
arch/aarch32/boot.S. Subsequent patches will improve the support and add
support for additional configurations.

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 | 14 ++++++++++++++
 arch/aarch32/psci.S |  5 -----
 arch/aarch64/boot.S | 13 +++++++++++++
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index 4add338..00c432d 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -16,6 +16,20 @@
 	.arch_extension virt
 
 	.section .init
+
+	/*
+	 * The boot-wrapper must be entered from the reset vector at the
+	 * highest implemented exception level. The boot-wrapper only supports
+	 * being entered in the following modes:
+	 *
+	 * - PL1 / EL3 (Secure) Supervisor mode
+	 *   Entering in this mode is strongly recommended.
+	 *   PL2 must be implemented.
+	 *
+	 * - PL2 / EL2 (Non-secure) Hypervisor mode
+	 *   Entering in this mode is partially supported.
+	 *   PSCI is not supported when entered in this mode.
+	 */
 ASM_FUNC(_start)
 	/* Stack initialisation */
 	cpuid	r0, r1
diff --git a/arch/aarch32/psci.S b/arch/aarch32/psci.S
index dc7aeb7..e0d2972 100644
--- a/arch/aarch32/psci.S
+++ b/arch/aarch32/psci.S
@@ -44,11 +44,6 @@ ASM_FUNC(start_el3)
 	/* pass through */
 
 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
-	 * sitting at EL3, and that this path is only taken by primary CPU.
-	 */
 	cpuid	r0, r1
 	blx	find_logical_id
 	b	psci_first_spin
diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index 27ba449..900b9f8 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -12,6 +12,19 @@
 
 	.section .init
 
+	/*
+	 * The boot-wrapper must be entered from the reset vector at the
+	 * highest implemented exception level. The boot-wrapper only supports
+	 * being entered at the following exception levels:
+	 *
+	 * - EL3 (Secure)
+	 *   Entering at EL3 is strongly recommended.
+	 *   EL2 must be implemented.
+	 *
+	 * - EL2 (Non-secure)
+	 *   Entering at EL2 is partially supported.
+	 *   PSCI is not supported when entered in this exception level.
+	 */
 ASM_FUNC(_start)
 	cpuid	x0, x1
 	bl	find_logical_id
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 02/13] Add bit-field macros
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 01/13] Document entry requirements Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 14:40   ` Andre Przywara
  2022-01-11 13:06 ` [bootwrapper PATCH 03/13] aarch64: add system register accessors Mark Rutland
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

Arm architectural documentation typically defines bit-fields as
`[msb,lsb]` and single-bit fields as `[bit]`. For clarity it would be
helpful if we could define fields in the same way.

Add helpers so that we can do so, along with helper to extract/insert
bit-field values.

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 include/bits.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 include/bits.h

diff --git a/include/bits.h b/include/bits.h
new file mode 100644
index 0000000..8824a38
--- /dev/null
+++ b/include/bits.h
@@ -0,0 +1,33 @@
+/*
+ * include/bits.h - helpers for bit-field definitions.
+ *
+ * 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.
+ */
+#ifndef __BITS_H
+#define __BITS_H
+
+#ifdef __ASSEMBLY__
+#define UL(x)	x
+#define ULL(x)	x
+#else
+#define UL(x)	x##UL
+#define ULL(x)	x##ULL
+#endif
+
+#define BITS(msb, lsb) \
+((~ULL(0) >> (63 - msb)) & (~ULL(0) << lsb))
+
+#define BIT(b)	BITS(b, b)
+
+#define BITS_LSB(bits)	(__builtin_ffsll(bits) - 1)
+
+#define BITS_EXTRACT(val, bits) \
+	(((val) & (bits)) >> BITS_LSB(bits))
+
+#define BITS_INSERT(bits, val) \
+	(((val) << BITS_LSB(bits)) & (bits))
+
+#endif
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 03/13] aarch64: add system register accessors
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 01/13] Document entry requirements Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 02/13] Add bit-field macros Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 04/13] aarch32: add coprocessor accessors Mark Rutland
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

We open code the use of mrs/msr for specific registers, which is
somewhat tedious. Add macros to do this generically, along with a helper
to extract a specific register field. Existing C usage is converted to
the new helpers, and register definitions moved to a common location.

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch64/include/asm/cpu.h    | 41 ++++++++++++++++++++++---------
 arch/aarch64/include/asm/gic-v3.h | 10 +++-----
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 63eb1c3..1053414 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -9,10 +9,14 @@
 #ifndef __ASM_AARCH64_CPU_H
 #define __ASM_AARCH64_CPU_H
 
+#include <bits.h>
+
 #define MPIDR_ID_BITS		0xff00ffffff
 
 #define CURRENTEL_EL3		(3 << 2)
 
+#define ID_AA64PFR0_EL1_GIC	BITS(27, 24)
+
 /*
  * RES1 bits,  little-endian, caches and MMU off, no alignment checking,
  * no WXN.
@@ -29,6 +33,12 @@
 
 #define CPTR_EL3_EZ		(1 << 8)
 
+#define ICC_SRE_EL2		S3_4_C12_C9_5
+#define ICC_SRE_EL3		S3_6_C12_C12_5
+#define ICC_CTLR_EL1		S3_0_C12_C12_4
+#define ICC_CTLR_EL3		S3_6_C12_C12_4
+#define ICC_PMR_EL1		S3_0_C4_C6_0
+
 #define ZCR_EL3			s3_6_c1_c2_0
 #define ZCR_EL3_LEN_MASK	0x1ff
 
@@ -50,20 +60,27 @@
 
 #define sevl()		asm volatile ("sevl\n" : : : "memory")
 
-static inline unsigned long read_mpidr(void)
-{
-	unsigned long mpidr;
+#define __str(def)	#def
 
-	asm volatile ("mrs	%0, mpidr_el1\n" : "=r" (mpidr));
-	return mpidr & MPIDR_ID_BITS;
-}
+#define mrs(reg)							\
+({									\
+	unsigned long __mrs_val;					\
+	asm volatile("mrs %0, " __str(reg) : "=r" (__mrs_val));		\
+	__mrs_val;							\
+})
 
-static inline uint64_t read_id_aa64pfr0(void)
-{
-	uint64_t val;
+#define msr(reg, val)							\
+do {									\
+	unsigned long __msr_val = val;					\
+	asm volatile("msr " __str(reg) ", %0" : : "r" (__msr_val));	\
+} while (0)
+
+#define mrs_field(reg, field) \
+	BITS_EXTRACT(mrs(reg), (reg##_##field))
 
-	asm volatile ("mrs	%0, id_aa64pfr0_el1\n" : "=r" (val));
-	return val;
+static inline unsigned long read_mpidr(void)
+{
+	return mrs(mpidr_el1) & MPIDR_ID_BITS;
 }
 
 static inline void iciallu(void)
@@ -73,7 +90,7 @@ static inline void iciallu(void)
 
 static inline int has_gicv3_sysreg(void)
 {
-	return !!((read_id_aa64pfr0() >> 24) & 0xf);
+	return !!mrs_field(ID_AA64PFR0_EL1, GIC);
 }
 
 #endif /* !__ASSEMBLY__ */
diff --git a/arch/aarch64/include/asm/gic-v3.h b/arch/aarch64/include/asm/gic-v3.h
index 5b32380..2447480 100644
--- a/arch/aarch64/include/asm/gic-v3.h
+++ b/arch/aarch64/include/asm/gic-v3.h
@@ -9,20 +9,16 @@
 #ifndef __ASM_AARCH64_GICV3_H
 #define __ASM_AARCH64_GICV3_H
 
-#define ICC_SRE_EL2	"S3_4_C12_C9_5"
-#define ICC_SRE_EL3	"S3_6_C12_C12_5"
-#define ICC_CTLR_EL1	"S3_0_C12_C12_4"
-#define ICC_CTLR_EL3	"S3_6_C12_C12_4"
-#define ICC_PMR_EL1	"S3_0_C4_C6_0"
+#include <asm/cpu.h>
 
 static inline void gic_write_icc_sre(uint32_t val)
 {
-	asm volatile ("msr " ICC_SRE_EL3 ", %0" : : "r" (val));
+	msr(ICC_SRE_EL3, val);
 }
 
 static inline void gic_write_icc_ctlr(uint32_t val)
 {
-	asm volatile ("msr " ICC_CTLR_EL3 ", %0" : : "r" (val));
+	msr(ICC_CTLR_EL3, val);
 }
 
 #endif
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 04/13] aarch32: add coprocessor accessors
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (2 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 03/13] aarch64: add system register accessors Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 05/13] aarch64: add mov_64 macro Mark Rutland
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

We open code the use of mrc/mcr for specific registers, which is
somewhat tedious. Add macros to do this generically, along with a helper
to extract a specific register field. Existing C usage is converted to
the new helpers, and register definitions moved to a common location.

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch32/include/asm/cpu.h    | 45 ++++++++++++++++++++-----------
 arch/aarch32/include/asm/gic-v3.h |  6 +++--
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h
index 105cae5..d691c7b 100644
--- a/arch/aarch32/include/asm/cpu.h
+++ b/arch/aarch32/include/asm/cpu.h
@@ -9,9 +9,13 @@
 #ifndef __ASM_AARCH32_CPU_H
 #define __ASM_AARCH32_CPU_H
 
+#include <bits.h>
+
 #define MPIDR_ID_BITS		0x00ffffff
 #define MPIDR_INVALID		(-1)
 
+#define ID_PFR1_GIC		BITS(31, 28)
+
 /* Only RES1 bits and CP15 barriers for the kernel */
 #define HSCTLR_KERNEL		(3 << 28 | 3 << 22 | 1 << 18 | 1 << 16 | 1 << 11 | 3 << 4)
 #define SCTLR_KERNEL		(3 << 22 | 1 << 11 | 1 << 5 | 3 << 4)
@@ -40,32 +44,43 @@
 #define sevl()		asm volatile ("sev" : : : "memory")
 #endif
 
-static inline unsigned long read_mpidr(void)
-{
-	unsigned long mpidr;
+#define MPIDR		"p15, 0, %0, c0, c0, 5"
+#define ID_PFR1		"p15, 0, %0, c0, c1, 1"
+#define ICIALLU		"p15, 0, %0, c7, c5, 0"
 
-	asm volatile ("mrc	p15, 0, %0, c0, c0, 5\n" : "=r" (mpidr));
-	return mpidr & MPIDR_ID_BITS;
-}
+#define ICC_SRE		"p15, 6, %0, c12, c12, 5"
+#define ICC_CTLR	"p15, 6, %0, c12, c12, 4"
 
-static inline uint32_t read_id_pfr1(void)
-{
-	uint32_t val;
+#define mrc(reg)						\
+({								\
+	unsigned long __mrc_val;				\
+	asm volatile("mrc " reg : "=r" (__mrc_val));		\
+	__mrc_val;						\
+})
 
-	asm volatile ("mrc	p15, 0, %0, c0, c1, 1\n" : "=r" (val));
-	return val;
+#define mcr(reg, val)						\
+do {								\
+	unsigned long __mcr_val = val;				\
+	asm volatile("mcr " reg : : "r" (__mcr_val));		\
+} while (0)
+
+
+#define mrc_field(reg, field) \
+	BITS_EXTRACT(mrc(reg), (reg##_##field))
+
+static inline unsigned long read_mpidr(void)
+{
+	return mrc(MPIDR) & MPIDR_ID_BITS;
 }
 
 static inline void iciallu(void)
 {
-	uint32_t val = 0;
-
-	asm volatile ("mcr	p15, 0, %0, c7, c5, 0" : : "r" (val));
+	mcr(ICIALLU, 0);
 }
 
 static inline int has_gicv3_sysreg(void)
 {
-	return !!((read_id_pfr1() >> 28) & 0xf);
+	return !!mrc_field(ID_PFR1, GIC);
 }
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/aarch32/include/asm/gic-v3.h b/arch/aarch32/include/asm/gic-v3.h
index 65f38de..b28136a 100644
--- a/arch/aarch32/include/asm/gic-v3.h
+++ b/arch/aarch32/include/asm/gic-v3.h
@@ -9,14 +9,16 @@
 #ifndef __ASM_AARCH32_GICV3_H
 #define __ASM_AARCH32_GICV3_H
 
+#include <asm/cpu.h>
+
 static inline void gic_write_icc_sre(uint32_t val)
 {
-	asm volatile ("mcr p15, 6, %0, c12, c12, 5" : : "r" (val));
+	mcr(ICC_SRE, val);
 }
 
 static inline void gic_write_icc_ctlr(uint32_t val)
 {
-	asm volatile ("mcr p15, 6, %0, c12, c12, 4" : : "r" (val));
+	mcr(ICC_CTLR, val);
 }
 
 #endif
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 05/13] aarch64: add mov_64 macro
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (3 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 04/13] aarch32: add coprocessor accessors Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 14:41   ` Andre Przywara
  2022-01-11 13:06 ` [bootwrapper PATCH 06/13] aarch64: initialize SCTLR_ELx for the boot-wrapper Mark Rutland
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

In subsequent patches we'll need to load 64-bit values into GPRs before
the CPU is in a known endianness, where we cannot use literal pools.

In preparation for that, this patch adds a new `mov_64` macro to load a
64-bit value into a GPR using a sequence of MOV and MOVKs, which will
function the same regardless of the CPU's endianness.

At the same time, move the `cpuid` macro to use `mov_64` internally.

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

diff --git a/arch/aarch64/common.S b/arch/aarch64/common.S
index c7171a9..3279fa9 100644
--- a/arch/aarch64/common.S
+++ b/arch/aarch64/common.S
@@ -9,9 +9,17 @@
 
 #include <cpu.h>
 
+	/* Load a 64-bit value using immediates */
+	.macro	mov_64 dest, val
+	mov	\dest, #(((\val) >>  0) & 0xffff)
+	movk	\dest, #(((\val) >> 16) & 0xffff), lsl #16
+	movk	\dest, #(((\val) >> 32) & 0xffff), lsl #32
+	movk	\dest, #(((\val) >> 48) & 0xffff), lsl #48
+	.endm
+
 	/* Put MPIDR into \dest, clobber \tmp and flags */
 	.macro cpuid dest, tmp
 	mrs	\dest, mpidr_el1
-	ldr	\tmp, =MPIDR_ID_BITS
+	mov_64	\tmp, MPIDR_ID_BITS
 	ands	\dest, \dest, \tmp
 	.endm
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 06/13] aarch64: initialize SCTLR_ELx for the boot-wrapper
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (4 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 05/13] aarch64: add mov_64 macro Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 14:38   ` Robin Murphy
  2022-01-11 13:06 ` [bootwrapper PATCH 07/13] Rework common init C code Mark Rutland
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

The SCTLR_ELx registers contain fields which are UNKNOWN or
IMPLEMENTATION DEFINED out of reset. This includes SCTLR_ELx.EE, which
defines the endianness of memory accesses (e.g. reads from literal
pools). Due to this, portions of boot-wrapper code are not guaranteed
top work correctly.

Rework the startup code to explicitly initialize SCTLR_ELx for the
exception level the boot-wrapper was entered at. When entered at EL2
it's necessary to first initialise HCR_EL2.E2H as this affects the RESx
behaviour of bits in SCTLR_EL2, and also aliases SCTLR_EL1 to SCTLR_EL2,
which would break the initialization performed in jump_kernel.

As we plan to eventually support the highest implemented EL being any of
EL3/EL2/EL1, code is added to handle all of these exception levels, even
though we do not currently support starting at EL1.

We'll initialize other registers in subsequent patches.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch64/boot.S            | 74 +++++++++++++++++++++++++++-------
 arch/aarch64/include/asm/cpu.h | 27 ++++++++++++-
 2 files changed, 85 insertions(+), 16 deletions(-)

diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index 900b9f8..45a0367 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -26,26 +26,26 @@
 	 *   PSCI is not supported when entered in this exception level.
 	 */
 ASM_FUNC(_start)
-	cpuid	x0, x1
-	bl	find_logical_id
-	cmp	x0, #MPIDR_INVALID
-	beq	err_invalid_id
-	bl	setup_stack
-
-	/*
-	 * EL3 initialisation
-	 */
 	mrs	x0, CurrentEL
 	cmp	x0, #CURRENTEL_EL3
-	b.eq	1f
+	b.eq	reset_at_el3
+	cmp	x0, #CURRENTEL_EL2
+	b.eq	reset_at_el2
+	cmp	x0, #CURRENTEL_EL1
+	b.eq	reset_at_el1
 
-	mov	w0, #1
-	ldr	x1, =flag_no_el3
-	str	w0, [x1]
+	/* Booting at EL0 is not supported */
+	b	.
 
-	b	start_no_el3
+	/*
+	 * EL3 initialisation
+	 */
+reset_at_el3:
+	mov_64	x0, SCTLR_EL3_RESET
+	msr	sctlr_el3, x0
+	isb
 
-1:	mov	x0, #0x30			// RES1
+	mov	x0, #0x30			// RES1
 	orr	x0, x0, #(1 << 0)		// Non-secure EL1
 	orr	x0, x0, #(1 << 8)		// HVC enable
 
@@ -135,10 +135,54 @@ ASM_FUNC(_start)
 	ldr	x0, =COUNTER_FREQ
 	msr	cntfrq_el0, x0
 
+	cpuid	x0, x1
+	bl	find_logical_id
+	cmp	x0, #MPIDR_INVALID
+	b.eq	err_invalid_id
+	bl	setup_stack
+
 	bl	gic_secure_init
 
 	b	start_el3
 
+	/*
+	 * EL2 initialization
+	 */
+reset_at_el2:
+	// Ensure E2H is not in use
+	mov_64	x0, HCR_EL2_RESET
+	msr	hcr_el2, x0
+	isb
+
+	mov_64	x0, SCTLR_EL2_RESET
+	msr	sctlr_el2, x0
+	isb
+
+	b	reset_no_el3
+
+	/*
+	 * EL1 initialization
+	 */
+reset_at_el1:
+	mov_64	x0, SCTLR_EL1_RESET
+	msr	sctlr_el1, x0
+	isb
+
+	b	reset_no_el3
+
+reset_no_el3:
+	cpuid	x0, x1
+	bl	find_logical_id
+	cmp	x0, #MPIDR_INVALID
+	b.eq	err_invalid_id
+	bl	setup_stack
+
+	mov	w0, #1
+	ldr	x1, =flag_no_el3
+	str	w0, [x1]
+
+	b	start_no_el3
+
 err_invalid_id:
 	b	.
 
diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 1053414..f0edd2a 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -14,6 +14,32 @@
 #define MPIDR_ID_BITS		0xff00ffffff
 
 #define CURRENTEL_EL3		(3 << 2)
+#define CURRENTEL_EL2		(2 << 2)
+#define CURRENTEL_EL1		(1 << 2)
+
+/*
+ * RES1 bit definitions definitions as of ARM DDI 0487G.b
+ *
+ * These includes bits which are RES1 in some configurations.
+ */
+#define SCTLR_EL3_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
+				 1 << 18 | 1 << 16 | 1 << 11 | 1 << 5 | 1 << 4)
+
+#define SCTLR_EL2_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
+				 1 << 18 | 1 << 16 | 1 << 11 | 1 << 5 | 1 << 4)
+
+#define SCTLR_EL1_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
+				 1 << 11 | 1 << 8 | 1 << 7 | 1 << 4)
+
+#define HCR_EL2_RES1		(1 << 1)
+
+/*
+ * Initial register values required for the boot-wrapper to run out-of-reset.
+ */
+#define SCTLR_EL3_RESET		SCTLR_EL3_RES1
+#define SCTLR_EL2_RESET		SCTLR_EL2_RES1
+#define SCTLR_EL1_RESET		SCTLR_EL1_RES1
+#define HCR_EL2_RESET		HCR_EL2_RES1
 
 #define ID_AA64PFR0_EL1_GIC	BITS(27, 24)
 
@@ -43,7 +69,6 @@
 #define ZCR_EL3_LEN_MASK	0x1ff
 
 #define SCTLR_EL1_CP15BEN	(1 << 5)
-#define SCTLR_EL1_RES1		(3 << 28 | 3 << 22 | 1 << 11)
 
 #ifdef KERNEL_32
 /* 32-bit kernel decompressor uses CP15 barriers */
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 07/13] Rework common init C code
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (5 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 06/13] aarch64: initialize SCTLR_ELx for the boot-wrapper Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 08/13] Announce boot-wrapper mode / exception level Mark Rutland
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

In init_platform() we initialize a UART and announce the presence of the
bootwrapper to the world. We do this relatively late in the boot-flow,
and prior to this will silently ignore errors (e.g. in gic_secure_init).

To make it possible to provide improved diagnostics, and to allow us to
move more initialization into C, this patch reworks the init code to
call a C function earlier, where we can announce the presence of the
boot-wrapper and perform other initialization.

In subsequent patches this will be expanded with more CPU
initialization.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 Makefile.am         |  2 +-
 arch/aarch32/boot.S |  5 +++++
 arch/aarch64/boot.S |  4 ++++
 common/boot.c       |  4 ----
 common/init.c       | 24 ++++++++++++++++++++++++
 common/platform.c   | 12 +++++++-----
 include/platform.h  | 17 +++++++++++++++++
 7 files changed, 58 insertions(+), 10 deletions(-)
 create mode 100644 common/init.c
 create mode 100644 include/platform.h

diff --git a/Makefile.am b/Makefile.am
index f941b07..0651c38 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -34,7 +34,7 @@ endif
 PSCI_CPU_OFF	:= 0x84000002
 
 COMMON_SRC	:= common/
-COMMON_OBJ	:= boot.o bakery_lock.o platform.o lib.o
+COMMON_OBJ	:= boot.o bakery_lock.o platform.o lib.o init.o
 
 ARCH_OBJ	:= boot.o stack.o utils.o
 
diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index 00c432d..ee073ea 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -48,6 +48,9 @@ ASM_FUNC(_start)
 	mov	r0, #1
 	ldr	r1, =flag_no_el3
 	str	r0, [r1]
+
+	bl	cpu_init_bootwrapper
+
 	b	start_no_el3
 
 _switch_monitor:
@@ -71,6 +74,8 @@ _monitor:
 	ldr	r0, =COUNTER_FREQ
 	mcr	p15, 0, r0, c14, c0, 0		@ CNTFRQ
 
+	bl	cpu_init_bootwrapper
+
 	bl	gic_secure_init
 
 	/* Initialise boot method */
diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index 45a0367..17b4a75 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -141,6 +141,8 @@ reset_at_el3:
 	b.eq	err_invalid_id
 	bl	setup_stack
 
+	bl	cpu_init_bootwrapper
+
 	bl	gic_secure_init
 
 	b	start_el3
@@ -181,6 +183,8 @@ reset_no_el3:
 	ldr	x1, =flag_no_el3
 	str	w0, [x1]
 
+	bl	cpu_init_bootwrapper
+
 	b	start_no_el3
 
 err_invalid_id:
diff --git a/common/boot.c b/common/boot.c
index c74d34c..29d53a4 100644
--- a/common/boot.c
+++ b/common/boot.c
@@ -12,8 +12,6 @@
 extern unsigned long entrypoint;
 extern unsigned long dtb;
 
-void init_platform(void);
-
 void __noreturn jump_kernel(unsigned long address,
 			    unsigned long a0,
 			    unsigned long a1,
@@ -62,8 +60,6 @@ void __noreturn first_spin(unsigned int cpu, unsigned long *mbox,
 			   unsigned long invalid)
 {
 	if (cpu == 0) {
-		init_platform();
-
 		*mbox = (unsigned long)&entrypoint;
 		sevl();
 		spin(mbox, invalid, 1);
diff --git a/common/init.c b/common/init.c
new file mode 100644
index 0000000..9c471c9
--- /dev/null
+++ b/common/init.c
@@ -0,0 +1,24 @@
+/*
+ * init.c - common boot-wrapper initialization
+ *
+ * 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.
+ */
+#include <cpu.h>
+#include <platform.h>
+
+static void announce_bootwrapper(void)
+{
+	print_string("Boot-wrapper v0.2\r\n\r\n");
+}
+
+void cpu_init_bootwrapper(void)
+{
+	if (this_cpu_logical_id() == 0) {
+		init_uart();
+		announce_bootwrapper();
+		init_platform();
+	}
+}
diff --git a/common/platform.c b/common/platform.c
index d11f568..47bf547 100644
--- a/common/platform.c
+++ b/common/platform.c
@@ -1,5 +1,5 @@
 /*
- * platform.c - code to initialise everything required when first booting.
+ * platform.c - Platform initialization and I/O.
  *
  * Copyright (C) 2015 ARM Limited. All rights reserved.
  *
@@ -7,6 +7,7 @@
  * found in the LICENSE.txt file.
  */
 
+#include <cpu.h>
 #include <stdint.h>
 
 #include <asm/io.h>
@@ -30,7 +31,7 @@
 #define V2M_SYS(reg)	((void *)SYSREGS_BASE + V2M_SYS_##reg)
 #endif
 
-static void print_string(const char *str)
+void print_string(const char *str)
 {
 	uint32_t flags;
 
@@ -47,7 +48,7 @@ static void print_string(const char *str)
 	}
 }
 
-void init_platform(void)
+void init_uart(void)
 {
 	/*
 	 * UART initialisation (38400 8N1)
@@ -58,9 +59,10 @@ void init_platform(void)
 	raw_writel(0x70,	PL011(UART_LCR_H));
 	/* Enable the UART, TXen and RXen */
 	raw_writel(0x301,	PL011(UARTCR));
+}
 
-	print_string("Boot-wrapper v0.2\r\n\r\n");
-
+void init_platform(void)
+{
 #ifdef SYSREGS_BASE
 	/*
 	 * CLCD output site MB
diff --git a/include/platform.h b/include/platform.h
new file mode 100644
index 0000000..e5248e1
--- /dev/null
+++ b/include/platform.h
@@ -0,0 +1,17 @@
+/*
+ * include/platform.h - Platform initialization and I/O.
+ *
+ * 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.
+ */
+#ifndef __PLATFORM_H
+#define __PLATFORM_H
+
+void print_string(const char *str);
+void init_uart(void);
+
+void init_platform(void);
+
+#endif /* __PLATFORM_H */
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 08/13] Announce boot-wrapper mode / exception level
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (6 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 07/13] Rework common init C code Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 09/13] aarch64: move the bulk of EL3 initialization to C Mark Rutland
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

When something goes wrong within the boot-wrapper, it can be very
helpful to know where we started from. Add an arch_announce() function
to log this early in the boot process. More information can be added
here in future.

This is logged ot the serial console as:

| Boot-wrapper v0.2
| Entered at EL3

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 Makefile.am                    |  2 +-
 arch/aarch32/include/asm/cpu.h |  9 +++++++++
 arch/aarch32/init.c            | 30 ++++++++++++++++++++++++++++++
 arch/aarch64/init.c            | 19 +++++++++++++++++++
 common/init.c                  |  6 +++++-
 common/platform.c              | 24 ++++++++++++++----------
 include/platform.h             |  1 +
 7 files changed, 79 insertions(+), 12 deletions(-)
 create mode 100644 arch/aarch32/init.c
 create mode 100644 arch/aarch64/init.c

diff --git a/Makefile.am b/Makefile.am
index 0651c38..885a77c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -36,7 +36,7 @@ PSCI_CPU_OFF	:= 0x84000002
 COMMON_SRC	:= common/
 COMMON_OBJ	:= boot.o bakery_lock.o platform.o lib.o init.o
 
-ARCH_OBJ	:= boot.o stack.o utils.o
+ARCH_OBJ	:= boot.o stack.o utils.o init.o
 
 if BOOTWRAPPER_32
 CPPFLAGS	+= -DBOOTWRAPPER_32
diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h
index d691c7b..aa72204 100644
--- a/arch/aarch32/include/asm/cpu.h
+++ b/arch/aarch32/include/asm/cpu.h
@@ -44,6 +44,15 @@
 #define sevl()		asm volatile ("sev" : : : "memory")
 #endif
 
+static inline unsigned long read_cpsr(void)
+{
+	unsigned long cpsr;
+	asm volatile ("mrs      %0, cpsr\n" : "=r" (cpsr));
+	return cpsr;
+}
+
+#define read_cpsr_mode()       (read_cpsr() & PSR_MODE_MASK)
+
 #define MPIDR		"p15, 0, %0, c0, c0, 5"
 #define ID_PFR1		"p15, 0, %0, c0, c1, 1"
 #define ICIALLU		"p15, 0, %0, c7, c5, 0"
diff --git a/arch/aarch32/init.c b/arch/aarch32/init.c
new file mode 100644
index 0000000..b29ebb4
--- /dev/null
+++ b/arch/aarch32/init.c
@@ -0,0 +1,30 @@
+/*
+ * init.c - common boot-wrapper initialization
+ *
+ * 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.
+ */
+#include <platform.h>
+
+#include <asm/cpu.h>
+
+static const char *mode_string(void)
+{
+	switch (read_cpsr_mode()) {
+	case PSR_MON:
+		return "PL1";
+	case PSR_HYP:
+		return "PL2 (Non-secure)";
+	default:
+		return "<UNKNOWN MODE>";
+	}
+}
+
+void announce_arch(void)
+{
+	print_string("Entered at ");
+	print_string(mode_string());
+	print_string("\r\n");
+}
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
new file mode 100644
index 0000000..82816e7
--- /dev/null
+++ b/arch/aarch64/init.c
@@ -0,0 +1,19 @@
+/*
+ * init.c - common boot-wrapper initialization
+ *
+ * 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.
+ */
+#include <cpu.h>
+#include <platform.h>
+
+void announce_arch(void)
+{
+	unsigned char el = mrs(CurrentEl) >> 2;
+
+	print_string("Entered at EL");
+	print_char('0' + el);
+	print_string("\r\n");
+}
diff --git a/common/init.c b/common/init.c
index 9c471c9..2600f73 100644
--- a/common/init.c
+++ b/common/init.c
@@ -11,14 +11,18 @@
 
 static void announce_bootwrapper(void)
 {
-	print_string("Boot-wrapper v0.2\r\n\r\n");
+	print_string("Boot-wrapper v0.2\r\n");
 }
 
+void announce_arch(void);
+
 void cpu_init_bootwrapper(void)
 {
 	if (this_cpu_logical_id() == 0) {
 		init_uart();
 		announce_bootwrapper();
+		announce_arch();
+		print_string("\r\n");
 		init_platform();
 	}
 }
diff --git a/common/platform.c b/common/platform.c
index 47bf547..80d0562 100644
--- a/common/platform.c
+++ b/common/platform.c
@@ -31,21 +31,25 @@
 #define V2M_SYS(reg)	((void *)SYSREGS_BASE + V2M_SYS_##reg)
 #endif
 
-void print_string(const char *str)
+void print_char(char c)
 {
 	uint32_t flags;
 
-	while (*str) {
-		do
-			flags = raw_readl(PL011(UARTFR));
-		while (flags & PL011_UARTFR_FIFO_FULL);
+	do {
+		flags = raw_readl(PL011(UARTFR));
+	} while (flags & PL011_UARTFR_FIFO_FULL);
+
+	raw_writel(c, PL011(UARTDR));
 
-		raw_writel(*str++, PL011(UARTDR));
+	do {
+		flags = raw_readl(PL011(UARTFR));
+	} while (flags & PL011_UARTFR_BUSY);
+}
 
-		do
-			flags = raw_readl(PL011(UARTFR));
-		while (flags & PL011_UARTFR_BUSY);
-	}
+void print_string(const char *str)
+{
+	while (*str)
+		print_char(*str++);
 }
 
 void init_uart(void)
diff --git a/include/platform.h b/include/platform.h
index e5248e1..237b481 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -9,6 +9,7 @@
 #ifndef __PLATFORM_H
 #define __PLATFORM_H
 
+void print_char(char c);
 void print_string(const char *str);
 void init_uart(void);
 
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 09/13] aarch64: move the bulk of EL3 initialization to C
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (7 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 08/13] Announce boot-wrapper mode / exception level Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 10/13] aarch32: move the bulk of Secure PL1 " Mark Rutland
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

The majority of state that we initialize at EL3 is necessary for code at
lower ELs to function, but isnt' necessary for the boot-wrapper itself.
Given that, it would be better to write this in C where it can be
written mode clearly, and where it will be possible to add logging/debug
logic.

This patch migrates the AArch64 EL3 initialization to C.

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 Makefile.am                    |  2 +-
 arch/aarch64/boot.S            | 92 +---------------------------------
 arch/aarch64/include/asm/cpu.h | 36 ++++++++++++-
 arch/aarch64/init.c            | 69 +++++++++++++++++++++++++
 4 files changed, 107 insertions(+), 92 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 885a77c..a598b95 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -153,7 +153,7 @@ $(COMMON_SRC):
 %.o: %.S Makefile | $(ARCH_SRC)
 	$(CC) $(CPPFLAGS) -D__ASSEMBLY__ $(CFLAGS) $(DEFINES) -c -o $@ $<
 
-%.o: %.c Makefile | $(COMMON_SRC)
+%.o: %.c Makefile | $(ARCH_SRC) $(COMMON_SRC)
 	$(CC) $(CPPFLAGS) $(CFLAGS) $(DEFINES) -c -o $@ $<
 
 model.lds: $(LD_SCRIPT) Makefile
diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index 17b4a75..c0ec518 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -45,96 +45,6 @@ reset_at_el3:
 	msr	sctlr_el3, x0
 	isb
 
-	mov	x0, #0x30			// RES1
-	orr	x0, x0, #(1 << 0)		// Non-secure EL1
-	orr	x0, x0, #(1 << 8)		// HVC enable
-
-	/* Enable pointer authentication if present */
-	mrs	x1, id_aa64isar1_el1
-	/* We check for APA+API and GPA+GPI */
-	ldr	x2, =((0xff << 24) | (0xff << 4))
-	and	x1, x1, x2
-	cbz	x1, 1f
-
-	orr	x0, x0, #(1 << 16)		// AP key enable
-	orr	x0, x0, #(1 << 17)		// AP insn enable
-1:
-	/* Enable TME if present */
-	mrs	x1, id_aa64isar0_el1
-	ubfx	x1, x1, #24, #4
-	cbz	x1, 1f
-
-	orr	x0, x0, #(1 << 34)		// TME enable
-1:
-	/* Enable FGT if present */
-	mrs	x1, id_aa64mmfr0_el1
-	ubfx	x1, x1, #56, #4
-	cbz	x1, 1f
-
-	orr	x0, x0, #(1 << 27)		// FGT enable
-1:
-	/* Enable ECV2 if present (allows CNTPOFF_EL2) */
-	mrs	x1, id_aa64mmfr0_el1
-	ubfx	x1, x1, #60, #4
-	cmp	x1, #2
-	b.lt	1f
-
-	orr	x0, x0, #(1 << 28)		// ECV enable
-1:
-	/* Enable MTE if present */
-	mrs	x10, id_aa64pfr1_el1
-	ubfx	x10, x10, #8, #4
-	cmp	x10, #2
-	b.lt	1f
-
-	orr	x0, x0, #(1 << 26)		// ATA enable
-1:
-#ifndef KERNEL_32
-	orr	x0, x0, #(1 << 10)		// 64-bit EL2
-#endif
-	msr	scr_el3, x0
-
-	msr	cptr_el3, xzr			// Disable copro. traps to EL3
-
-	mov	x0, xzr
-	mrs	x1, id_aa64dfr0_el1
-	ubfx	x1, x1, #32, #4
-	cbz	x1, 1f
-
-	// Enable SPE for the non-secure world.
-	orr	x0, x0, #(0x3 << 12)
-
-	// Do not trap PMSNEVFR_EL1 if present
-	cmp	x1, #3
-	b.lt	1f
-	orr	x0, x0, #(1 << 36)
-
-1:	mrs	x1, id_aa64dfr0_el1
-	ubfx	x1, x1, #44, #4
-	cbz	x1, 1f
-
-	// Enable TRBE for the non-secure world.
-	ldr	x1, =(0x3 << 24)
-	orr	x0, x0, x1
-
-1:	msr	mdcr_el3, x0			// Disable traps to EL3
-
-	mrs	x0, id_aa64pfr0_el1
-	ubfx	x0, x0, #32, #4			// SVE present?
-	cbz	x0, 1f				// Skip SVE init if not
-
-	mrs	x0, cptr_el3
-	orr	x0, x0, #CPTR_EL3_EZ		// enable SVE
-	msr	cptr_el3, x0
-	isb
-
-	mov	x0, #ZCR_EL3_LEN_MASK		// SVE: Enable full vector len
-	msr	ZCR_EL3, x0			// for EL2.
-
-1:
-	ldr	x0, =COUNTER_FREQ
-	msr	cntfrq_el0, x0
-
 	cpuid	x0, x1
 	bl	find_logical_id
 	cmp	x0, #MPIDR_INVALID
@@ -143,6 +53,8 @@ reset_at_el3:
 
 	bl	cpu_init_bootwrapper
 
+	bl	cpu_init_el3
+
 	bl	gic_secure_init
 
 	b	start_el3
diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index f0edd2a..5afcf78 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -31,7 +31,41 @@
 #define SCTLR_EL1_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
 				 1 << 11 | 1 << 8 | 1 << 7 | 1 << 4)
 
-#define HCR_EL2_RES1		(1 << 1)
+#define ZCR_EL3				s3_6_c1_c2_0
+#define ZCR_EL3_LEN			BITS(3, 1)
+
+#define MDCR_EL3_NSPB_NS_NOTRAP		(UL(3) << 12)
+#define MDCR_EL3_NSTB_NS_NOTRAP		(UL(3) << 24)
+#define MDCR_EL3_ENPMSN			BIT(36)
+
+#define SCR_EL3_RES1			BITS(5, 4)
+#define SCR_EL3_NS			BIT(0)
+#define SCR_EL3_HCE			BIT(8)
+#define SCR_EL3_RW			BIT(10)
+#define SCR_EL3_APK			BIT(16)
+#define SCR_EL3_API			BIT(17)
+#define SCR_EL3_ATA			BIT(26)
+#define SCR_EL3_FGTEN			BIT(27)
+#define SCR_EL3_ECVEN			BIT(28)
+#define SCR_EL3_TME			BIT(34)
+
+#define HCR_EL2_RES1			BIT(1)
+
+#define ID_AA64DFR0_EL1_PMSVER		BITS(35, 32)
+#define ID_AA64DFR0_EL1_TRACEBUFFER	BITS(47, 44)
+
+#define ID_AA64ISAR0_EL1_TME		BITS(27, 24)
+
+#define ID_AA64ISAR1_EL1_APA		BITS(7, 4)
+#define ID_AA64ISAR1_EL1_API		BITS(11, 8)
+#define ID_AA64ISAR1_EL1_GPA		BITS(27, 24)
+#define ID_AA64ISAR1_EL1_GPI		BITS(31, 28)
+
+#define ID_AA64MMFR0_EL1_FGT		BITS(59, 56)
+#define ID_AA64MMFR0_EL1_ECV		BITS(63, 60)
+
+#define ID_AA64PFR1_EL1_MTE		BITS(11, 8)
+#define ID_AA64PFR0_EL1_SVE		BITS(35, 32)
 
 /*
  * Initial register values required for the boot-wrapper to run out-of-reset.
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index 82816e7..ded9871 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -8,6 +8,7 @@
  */
 #include <cpu.h>
 #include <platform.h>
+#include <stdbool.h>
 
 void announce_arch(void)
 {
@@ -17,3 +18,71 @@ void announce_arch(void)
 	print_char('0' + el);
 	print_string("\r\n");
 }
+
+static inline bool kernel_is_32bit(void)
+{
+#ifdef KERNEL_32
+	return true;
+#else
+	return false;
+#endif
+}
+
+static inline bool cpu_has_pauth(void)
+{
+	const unsigned long id_pauth = ID_AA64ISAR1_EL1_APA |
+				       ID_AA64ISAR1_EL1_API |
+				       ID_AA64ISAR1_EL1_GPA |
+				       ID_AA64ISAR1_EL1_GPI;
+
+	return mrs(ID_AA64ISAR1_EL1) & id_pauth;
+}
+
+void cpu_init_el3(void)
+{
+	unsigned long scr = SCR_EL3_RES1 | SCR_EL3_NS | SCR_EL3_HCE;
+	unsigned long mdcr = 0;
+	unsigned long cptr = 0;
+
+	if (cpu_has_pauth())
+		scr |= SCR_EL3_APK | SCR_EL3_API;
+
+	if (mrs_field(ID_AA64ISAR0_EL1, TME))
+		scr |= SCR_EL3_TME;
+
+	if (mrs_field(ID_AA64MMFR0_EL1, FGT))
+		scr |= SCR_EL3_FGTEN;
+
+	if (mrs_field(ID_AA64MMFR0_EL1, ECV) >= 2)
+		scr |= SCR_EL3_ECVEN;
+
+	if (mrs_field(ID_AA64PFR1_EL1, MTE))
+		scr |= SCR_EL3_ATA;
+
+	if (!kernel_is_32bit())
+		scr |= SCR_EL3_RW;
+
+	msr(SCR_EL3, scr);
+
+	msr(CPTR_EL3, cptr);
+
+	if (mrs_field(ID_AA64DFR0_EL1, PMSVER))
+		mdcr |= MDCR_EL3_NSPB_NS_NOTRAP;
+
+	if (mrs_field(ID_AA64DFR0_EL1, PMSVER) >= 3)
+		mdcr |= MDCR_EL3_ENPMSN;
+
+	if (mrs_field(ID_AA64DFR0_EL1, TRACEBUFFER))
+		mdcr |= MDCR_EL3_NSTB_NS_NOTRAP;
+
+	msr(MDCR_EL3, mdcr);
+
+	if (mrs_field(ID_AA64PFR0_EL1, SVE)) {
+		cptr |= CPTR_EL3_EZ;
+		msr(CPTR_EL3, cptr);
+		isb();
+		msr(ZCR_EL3, ZCR_EL3_LEN);
+	}
+
+	msr(CNTFRQ_EL0, COUNTER_FREQ);
+}
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 10/13] aarch32: move the bulk of Secure PL1 initialization to C
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (8 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 09/13] aarch64: move the bulk of EL3 initialization to C Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 11/13] Announce locations of memory objects Mark Rutland
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

The majority of state that we initialize at Secure PL1 is necessary for
code at lower PLs to function, but isnt' necessary for the boot-wrapper
itself.  Given that, it would be better to write this in C where it can
be written mode clearly, and where it will be possible to add
logging/debug logic.

This patch migrates the AArch32 Secure PL1 initialization to C.

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            | 11 +----------
 arch/aarch32/include/asm/cpu.h |  9 +++++++++
 arch/aarch32/init.c            | 12 ++++++++++++
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index ee073ea..820957b 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -63,16 +63,7 @@ _monitor:
 	/* Move the stack to Monitor mode*/
 	mrs	sp, sp_svc
 
-	/* Setup secure registers and devices */
-	mov	r0, #1				@ Non-secure lower level
-	orr	r0, #(1 << 8)			@ HVC enable
-	mcr	p15, 0, r0, c1, c1, 0		@ SCR
-
-	mov	r0, #(1 << 10 | 1 << 11)	@ Enable NS access to CPACR
-	mcr	p15, 0, r0, c1, c1, 2		@ NSACR
-
-	ldr	r0, =COUNTER_FREQ
-	mcr	p15, 0, r0, c14, c0, 0		@ CNTFRQ
+	bl	cpu_init_secure_pl1
 
 	bl	cpu_init_bootwrapper
 
diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h
index aa72204..c1bce9a 100644
--- a/arch/aarch32/include/asm/cpu.h
+++ b/arch/aarch32/include/asm/cpu.h
@@ -30,6 +30,11 @@
 #define PSR_I			(1 << 7)
 #define PSR_A			(1 << 8)
 
+#define SCR_NS			BIT(0)
+#define SCR_HCE			BIT(8)
+
+#define NSACR_CP10		BIT(10)
+#define NSACR_CP11		BIT(11)
 
 #define SPSR_KERNEL		(PSR_A | PSR_I | PSR_F | PSR_HYP)
 
@@ -55,11 +60,15 @@ static inline unsigned long read_cpsr(void)
 
 #define MPIDR		"p15, 0, %0, c0, c0, 5"
 #define ID_PFR1		"p15, 0, %0, c0, c1, 1"
+#define SCR		"p15, 0, %0, c1, c1, 0"
+#define NSACR		"p15, 0, %0, c1, c1, 2"
 #define ICIALLU		"p15, 0, %0, c7, c5, 0"
 
 #define ICC_SRE		"p15, 6, %0, c12, c12, 5"
 #define ICC_CTLR	"p15, 6, %0, c12, c12, 4"
 
+#define CNTFRQ		"p15, 0, %0, c14, c0, 0"
+
 #define mrc(reg)						\
 ({								\
 	unsigned long __mrc_val;				\
diff --git a/arch/aarch32/init.c b/arch/aarch32/init.c
index b29ebb4..5b69dcd 100644
--- a/arch/aarch32/init.c
+++ b/arch/aarch32/init.c
@@ -28,3 +28,15 @@ void announce_arch(void)
 	print_string(mode_string());
 	print_string("\r\n");
 }
+
+void cpu_init_secure_pl1(void)
+{
+	unsigned long scr = SCR_NS | SCR_HCE;
+	unsigned long nsacr = NSACR_CP10 | NSACR_CP11;
+
+	mcr(SCR, scr);
+
+	mcr(NSACR, nsacr);
+
+	mcr(CNTFRQ, COUNTER_FREQ);
+}
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 11/13] Announce locations of memory objects
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (9 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 10/13] aarch32: move the bulk of Secure PL1 " Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-14 10:48   ` Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 12/13] Rework bootmethod initialization Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 13/13] Unify start_el3 & start_no_el3 Mark Rutland
  12 siblings, 1 reply; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

To make it easier to debug boot failures, log the location of memory
objects at boot time.

This is logged to the serial console as:

| Boot-wrapper v0.2
| Entered at EL3
| Memory layout:
| [0000000080000000..0000000080001f90] => boot-wrapper
| [000000008000fff8..0000000080010000] => mbox
| [0000000080200000..00000000822af200] => kernel
| [0000000088000000..0000000088002857] => dtb

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 common/init.c      | 27 +++++++++++++++++++++++++++
 common/platform.c  | 14 ++++++++++++++
 include/platform.h |  2 ++
 model.lds.S        | 20 ++++++++++++++------
 4 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/common/init.c b/common/init.c
index 2600f73..fc74b9e 100644
--- a/common/init.c
+++ b/common/init.c
@@ -14,6 +14,32 @@ static void announce_bootwrapper(void)
 	print_string("Boot-wrapper v0.2\r\n");
 }
 
+#define announce_object(object, desc)				\
+do {								\
+	extern char object##__start[];				\
+	extern char object##__end[];				\
+	print_string("[");					\
+	print_ulong_hex((unsigned long)object##__start);	\
+	print_string("..");					\
+	print_ulong_hex((unsigned long)object##__end);		\
+	print_string("] => " desc "\r\n");			\
+} while (0)
+
+static void announce_objects(void)
+{
+	print_string("Memory layout:\r\n");
+	announce_object(text, "boot-wrapper");
+	announce_object(mbox, "mbox");
+	announce_object(kernel, "kernel");
+#ifdef XEN
+	announce_object(xen, "xen");
+#endif
+	announce_object(dtb, "dtb");
+#ifdef USE_INITRD
+	announce_object(filesystem, "initrd");
+#endif
+}
+
 void announce_arch(void);
 
 void cpu_init_bootwrapper(void)
@@ -22,6 +48,7 @@ void cpu_init_bootwrapper(void)
 		init_uart();
 		announce_bootwrapper();
 		announce_arch();
+		announce_objects();
 		print_string("\r\n");
 		init_platform();
 	}
diff --git a/common/platform.c b/common/platform.c
index 80d0562..5a91349 100644
--- a/common/platform.c
+++ b/common/platform.c
@@ -52,6 +52,20 @@ void print_string(const char *str)
 		print_char(*str++);
 }
 
+#define HEX_CHARS_PER_LONG (2 * sizeof(long))
+#define NIBBLES_PER
+
+void print_ulong_hex(unsigned long val)
+{
+	const char hex_chars[16] = "0123456789abcdef";
+	int i;
+
+	for (i = HEX_CHARS_PER_LONG - 1; i >= 0; i--) {
+		int v = (val >> (4 * i)) & 0xf;
+		print_char(hex_chars[v]);
+	}
+}
+
 void init_uart(void)
 {
 	/*
diff --git a/include/platform.h b/include/platform.h
index 237b481..c88e124 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -11,6 +11,8 @@
 
 void print_char(char c);
 void print_string(const char *str);
+void print_ulong_hex(unsigned long val);
+
 void init_uart(void);
 
 void init_platform(void);
diff --git a/model.lds.S b/model.lds.S
index d4e7e13..dacaa25 100644
--- a/model.lds.S
+++ b/model.lds.S
@@ -35,46 +35,54 @@ SECTIONS
 	 * the boot section's *(.data)
 	 */
 	.kernel (PHYS_OFFSET + KERNEL_OFFSET): {
-		kernel = .;
+		kernel__start = .;
 		KERNEL
+		kernel__end = .;
 	}
 
 #ifdef XEN
 	.xen (PHYS_OFFSET + XEN_OFFSET): {
-		xen = .;
+		xen__start = .;
 		XEN
+		xen__end = .;
 	}
 
-	entrypoint = xen;
+	entrypoint = xen__start;
 #else
-	entrypoint = kernel;
+	entrypoint = kernel__start;
 #endif
 
 	.dtb (PHYS_OFFSET + FDT_OFFSET): {
+		dtb__start = .;
 		dtb = .;
 		./fdt.dtb
+		dtb__end = .;
 	}
 
 #ifdef USE_INITRD
 	.filesystem (PHYS_OFFSET + FS_OFFSET): {
-		filesystem = .;
+		filesystem__start = .;
 		FILESYSTEM
-		fs_size = . - filesystem;
+		filesystem__end = .;
 	}
 #endif
 
 	.boot PHYS_OFFSET: {
+		text__start = .;
 		*(.init)
 		*(.text*)
 		*(.data* .rodata* .bss* COMMON)
 		*(.vectors)
 		*(.stack)
 		PROVIDE(etext = .);
+		text__end = .;
 	}
 
 	.mbox (PHYS_OFFSET + MBOX_OFFSET): {
+		mbox__start = .;
 		mbox = .;
 		QUAD(0x0)
+		mbox__end = .;
 	}
 
 	ASSERT(etext <= (PHYS_OFFSET + TEXT_LIMIT), ".text overflow!")
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 12/13] Rework bootmethod initialization
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (10 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 11/13] Announce locations of memory objects Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 13:06 ` [bootwrapper PATCH 13/13] Unify start_el3 & start_no_el3 Mark Rutland
  12 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

We currently initialize the bootmethod late, in assembly code. This
requires us to maintain the el3/no_el3 distintion late into the boot
process, and means we cannot produce any helpful diagnostic when booted
at an unexpected exception level.

Rework things so that we initialize the bootmethod early, with a warning
when things are wrong. The el3/no_el3 distinction is now irrelevant to
the bootmethod code, and can be removed in subsequent patches.

When a boot-wrapper configured for PSCI is entered at EL2, a warning is
looged to the serial console as:

| Boot-wrapper v0.2
| Entered at EL2
| Memory layout:
| [0000000080000000..0000000080001f90] => boot-wrapper
| [000000008000fff8..0000000080010000] => mbox
| [0000000080200000..00000000822af200] => kernel
| [0000000088000000..0000000088002857] => dtb
|
| WARNING: PSCI could not be initialized. Boot may fail

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/aarch32/include/asm/cpu.h  |  1 +
 arch/aarch32/include/asm/psci.h | 28 ++++++++++++++++++++++++++++
 arch/aarch32/psci.S             |  8 +-------
 arch/aarch32/utils.S            |  9 ---------
 arch/aarch64/include/asm/psci.h | 28 ++++++++++++++++++++++++++++
 arch/aarch64/psci.S             | 21 ++-------------------
 arch/aarch64/spin.S             |  3 +++
 arch/aarch64/utils.S            |  9 ---------
 common/init.c                   |  7 ++++++-
 common/psci.c                   | 22 +++++++++++++++++++---
 include/boot.h                  |  2 ++
 11 files changed, 90 insertions(+), 48 deletions(-)
 create mode 100644 arch/aarch32/include/asm/psci.h
 create mode 100644 arch/aarch64/include/asm/psci.h

diff --git a/arch/aarch32/include/asm/cpu.h b/arch/aarch32/include/asm/cpu.h
index c1bce9a..3426075 100644
--- a/arch/aarch32/include/asm/cpu.h
+++ b/arch/aarch32/include/asm/cpu.h
@@ -63,6 +63,7 @@ static inline unsigned long read_cpsr(void)
 #define SCR		"p15, 0, %0, c1, c1, 0"
 #define NSACR		"p15, 0, %0, c1, c1, 2"
 #define ICIALLU		"p15, 0, %0, c7, c5, 0"
+#define MVBAR		"p15, 0, %0, c12, c0, 1"
 
 #define ICC_SRE		"p15, 6, %0, c12, c12, 5"
 #define ICC_CTLR	"p15, 6, %0, c12, c12, 4"
diff --git a/arch/aarch32/include/asm/psci.h b/arch/aarch32/include/asm/psci.h
new file mode 100644
index 0000000..50c7c70
--- /dev/null
+++ b/arch/aarch32/include/asm/psci.h
@@ -0,0 +1,28 @@
+/*
+ * arch/aarch64/include/asm/psci.h
+ *
+ * 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.
+ */
+#ifndef __ASM_AARCH64_PSCI_H
+#define __ASM_AARCH64_PSCI_H
+
+#include <cpu.h>
+#include <stdbool.h>
+
+extern char psci_vectors[];
+
+static inline bool cpu_init_psci_arch(void)
+{
+	if (read_cpsr_mode() != PSR_MON)
+		return false;
+
+	mcr(MVBAR, (unsigned long)psci_vectors);
+	isb();
+
+	return true;
+}
+
+#endif
diff --git a/arch/aarch32/psci.S b/arch/aarch32/psci.S
index e0d2972..cdc36b0 100644
--- a/arch/aarch32/psci.S
+++ b/arch/aarch32/psci.S
@@ -15,7 +15,7 @@
 
 	.section .vectors
 	.align 6
-smc_vectors:
+ASM_DATA(psci_vectors)
 	b	err_exception			@ Reset
 	b	err_exception			@ Undef
 	b	handle_smc			@ SMC
@@ -39,11 +39,5 @@ handle_smc:
 	movs	pc, lr
 
 ASM_FUNC(start_el3)
-	ldr	r0, =smc_vectors
-	blx	setup_vector
-	/* pass through */
-
 ASM_FUNC(start_no_el3)
-	cpuid	r0, r1
-	blx	find_logical_id
 	b	psci_first_spin
diff --git a/arch/aarch32/utils.S b/arch/aarch32/utils.S
index 5809f48..58279aa 100644
--- a/arch/aarch32/utils.S
+++ b/arch/aarch32/utils.S
@@ -35,12 +35,3 @@ ASM_FUNC(find_logical_id)
 	bx	lr
 3:	mov	r0, #MPIDR_INVALID
 	bx	lr
-
-/*
- * Setup EL3 vectors.
- * r0: vector address
- */
-ASM_FUNC(setup_vector)
-	mcr	p15, 0, r0, c12, c0, 1	@ MVBAR
-	isb
-	bx	lr
diff --git a/arch/aarch64/include/asm/psci.h b/arch/aarch64/include/asm/psci.h
new file mode 100644
index 0000000..491e685
--- /dev/null
+++ b/arch/aarch64/include/asm/psci.h
@@ -0,0 +1,28 @@
+/*
+ * arch/aarch64/include/asm/psci.h
+ *
+ * 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.
+ */
+#ifndef __ASM_AARCH64_PSCI_H
+#define __ASM_AARCH64_PSCI_H
+
+#include <cpu.h>
+#include <stdbool.h>
+
+extern char psci_vectors[];
+
+static inline bool cpu_init_psci_arch(void)
+{
+	if (mrs(CurrentEL) != CURRENTEL_EL3)
+		return false;
+
+	msr(VBAR_EL3, (unsigned long)psci_vectors);
+	isb();
+
+	return true;
+}
+
+#endif
diff --git a/arch/aarch64/psci.S b/arch/aarch64/psci.S
index 8bd224b..d6ca2eb 100644
--- a/arch/aarch64/psci.S
+++ b/arch/aarch64/psci.S
@@ -19,7 +19,7 @@
 	.section .vectors, "w"
 
 	.align 11
-vector:
+ASM_DATA(psci_vectors)
 	// current EL, SP_EL0
 	ventry	err_exception	// synchronous
 	ventry	err_exception	// IRQ
@@ -80,22 +80,5 @@ smc_exit:
 	eret
 
 ASM_FUNC(start_el3)
-	ldr	x0, =vector
-	bl	setup_vector
-
-	/* only boot the primary cpu (entry 0 in the table) */
-	cpuid	x0, x1
-	bl	find_logical_id
-	b	psci_first_spin
-
-/*
- * This PSCI implementation requires EL3. Without EL3 we'll only boot the
- * primary cpu, all others will be trapped in an infinite loop.
- */
 ASM_FUNC(start_no_el3)
-	cpuid	x0, x1
-	bl	find_logical_id
-	cbz	x0, psci_first_spin
-spin_dead:
-	wfe
-	b	spin_dead
+	b	psci_first_spin
diff --git a/arch/aarch64/spin.S b/arch/aarch64/spin.S
index 1ea1c0b..764c532 100644
--- a/arch/aarch64/spin.S
+++ b/arch/aarch64/spin.S
@@ -12,6 +12,9 @@
 
 	.text
 
+ASM_FUNC(cpu_init_bootmethod)
+	ret
+
 ASM_FUNC(start_el3)
 ASM_FUNC(start_no_el3)
 	cpuid	x0, x1
diff --git a/arch/aarch64/utils.S b/arch/aarch64/utils.S
index 85c7f8a..32393cc 100644
--- a/arch/aarch64/utils.S
+++ b/arch/aarch64/utils.S
@@ -32,12 +32,3 @@ ASM_FUNC(find_logical_id)
 	ret
 3:	mov	x0, #MPIDR_INVALID
 	ret
-
-/*
- * Setup EL3 vectors
- * x0: vector address
- */
-ASM_FUNC(setup_vector)
-	msr	VBAR_EL3, x0
-	isb
-	ret
diff --git a/common/init.c b/common/init.c
index fc74b9e..3c05ac3 100644
--- a/common/init.c
+++ b/common/init.c
@@ -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 <boot.h>
 #include <cpu.h>
 #include <platform.h>
 
@@ -44,7 +45,9 @@ void announce_arch(void);
 
 void cpu_init_bootwrapper(void)
 {
-	if (this_cpu_logical_id() == 0) {
+	unsigned int cpu = this_cpu_logical_id();
+
+	if (cpu == 0) {
 		init_uart();
 		announce_bootwrapper();
 		announce_arch();
@@ -52,4 +55,6 @@ void cpu_init_bootwrapper(void)
 		print_string("\r\n");
 		init_platform();
 	}
+
+	cpu_init_bootmethod(cpu);
 }
diff --git a/common/psci.c b/common/psci.c
index a0e8700..8af6870 100644
--- a/common/psci.c
+++ b/common/psci.c
@@ -12,8 +12,11 @@
 #include <bakery_lock.h>
 #include <boot.h>
 #include <cpu.h>
+#include <platform.h>
 #include <psci.h>
 
+#include <asm/psci.h>
+
 #ifndef CPU_IDS
 #error "No MPIDRs provided"
 #endif
@@ -78,12 +81,25 @@ long psci_call(unsigned long fid, unsigned long arg1, unsigned long arg2)
 	}
 }
 
-void __noreturn psci_first_spin(unsigned int cpu)
+void __noreturn psci_first_spin(void)
 {
-	if (cpu == MPIDR_INVALID)
-		while (1);
+	unsigned int cpu = this_cpu_logical_id();
 
 	first_spin(cpu, branch_table + cpu, PSCI_ADDR_INVALID);
 
 	unreachable();
 }
+
+void cpu_init_bootmethod(unsigned int cpu)
+{
+	if (cpu_init_psci_arch())
+		return;
+
+	if (cpu == 0) {
+		print_string("WARNING: PSCI could not be initialized. Boot may fail\r\n\r\n");
+		return;
+	}
+
+	while (1)
+		wfe();
+}
diff --git a/include/boot.h b/include/boot.h
index d75e013..10968f8 100644
--- a/include/boot.h
+++ b/include/boot.h
@@ -16,4 +16,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_addr);
 
+void cpu_init_bootmethod(unsigned int cpu);
+
 #endif
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* [bootwrapper PATCH 13/13] Unify start_el3 & start_no_el3
  2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
                   ` (11 preceding siblings ...)
  2022-01-11 13:06 ` [bootwrapper PATCH 12/13] Rework bootmethod initialization Mark Rutland
@ 2022-01-11 13:06 ` Mark Rutland
  2022-01-11 14:39   ` Robin Murphy
  12 siblings, 1 reply; 25+ messages in thread
From: Mark Rutland @ 2022-01-11 13:06 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, mark.rutland, Wei.Chen

Now that the start_el3 and start_no_el3 labels point at the same place,
unify them into a start_bootmethod label and update callers.

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 | 5 ++---
 arch/aarch64/boot.S | 4 ++--
 arch/aarch64/psci.S | 3 +--
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
index 820957b..4d16c9c 100644
--- a/arch/aarch32/boot.S
+++ b/arch/aarch32/boot.S
@@ -51,7 +51,7 @@ ASM_FUNC(_start)
 
 	bl	cpu_init_bootwrapper
 
-	b	start_no_el3
+	b	start_bootmethod
 
 _switch_monitor:
 	adr	lr, _monitor
@@ -69,8 +69,7 @@ _monitor:
 
 	bl	gic_secure_init
 
-	/* Initialise boot method */
-	b	start_el3
+	b	start_bootmethod
 
 err_invalid_id:
 	b	.
diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
index c0ec518..da5fa65 100644
--- a/arch/aarch64/boot.S
+++ b/arch/aarch64/boot.S
@@ -57,7 +57,7 @@ reset_at_el3:
 
 	bl	gic_secure_init
 
-	b	start_el3
+	b	start_bootmethod
 
 	/*
 	 * EL2 initialization
@@ -97,7 +97,7 @@ reset_no_el3:
 
 	bl	cpu_init_bootwrapper
 
-	b	start_no_el3
+	b	start_bootmethod
 
 err_invalid_id:
 	b	.
diff --git a/arch/aarch64/psci.S b/arch/aarch64/psci.S
index d6ca2eb..9709dbb 100644
--- a/arch/aarch64/psci.S
+++ b/arch/aarch64/psci.S
@@ -79,6 +79,5 @@ smc_exit:
 	ldp	x18, x19, [sp], #16
 	eret
 
-ASM_FUNC(start_el3)
-ASM_FUNC(start_no_el3)
+ASM_FUNC(start_bootmethod)
 	b	psci_first_spin
-- 
2.30.2


_______________________________________________
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] 25+ messages in thread

* Re: [bootwrapper PATCH 06/13] aarch64: initialize SCTLR_ELx for the boot-wrapper
  2022-01-11 13:06 ` [bootwrapper PATCH 06/13] aarch64: initialize SCTLR_ELx for the boot-wrapper Mark Rutland
@ 2022-01-11 14:38   ` Robin Murphy
  2022-01-12 14:34     ` Mark Rutland
  0 siblings, 1 reply; 25+ messages in thread
From: Robin Murphy @ 2022-01-11 14:38 UTC (permalink / raw)
  To: Mark Rutland, linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, Wei.Chen

On 2022-01-11 13:06, Mark Rutland wrote:
> The SCTLR_ELx registers contain fields which are UNKNOWN or
> IMPLEMENTATION DEFINED out of reset. This includes SCTLR_ELx.EE, which
> defines the endianness of memory accesses (e.g. reads from literal
> pools). Due to this, portions of boot-wrapper code are not guaranteed
> top work correctly.

Nit: "to"

> Rework the startup code to explicitly initialize SCTLR_ELx for the
> exception level the boot-wrapper was entered at. When entered at EL2
> it's necessary to first initialise HCR_EL2.E2H as this affects the RESx
> behaviour of bits in SCTLR_EL2, and also aliases SCTLR_EL1 to SCTLR_EL2,
> which would break the initialization performed in jump_kernel.
> 
> As we plan to eventually support the highest implemented EL being any of
> EL3/EL2/EL1, code is added to handle all of these exception levels, even
> though we do not currently support starting at EL1.
> 
> We'll initialize other registers in subsequent patches.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>

[...]

> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index 1053414..f0edd2a 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -14,6 +14,32 @@
>   #define MPIDR_ID_BITS		0xff00ffffff
>   
>   #define CURRENTEL_EL3		(3 << 2)
> +#define CURRENTEL_EL2		(2 << 2)
> +#define CURRENTEL_EL1		(1 << 2)
> +
> +/*
> + * RES1 bit definitions definitions as of ARM DDI 0487G.b
> + *
> + * These includes bits which are RES1 in some configurations.
> + */
> +#define SCTLR_EL3_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
> +				 1 << 18 | 1 << 16 | 1 << 11 | 1 << 5 | 1 << 4)
> +
> +#define SCTLR_EL2_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
> +				 1 << 18 | 1 << 16 | 1 << 11 | 1 << 5 | 1 << 4)
> +
> +#define SCTLR_EL1_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
> +				 1 << 11 | 1 << 8 | 1 << 7 | 1 << 4)
> +
> +#define HCR_EL2_RES1		(1 << 1)

Maybe use the new BIT() macro in these? I reckon that would be more 
readable overall.

Robin.

> +
> +/*
> + * Initial register values required for the boot-wrapper to run out-of-reset.
> + */
> +#define SCTLR_EL3_RESET		SCTLR_EL3_RES1
> +#define SCTLR_EL2_RESET		SCTLR_EL2_RES1
> +#define SCTLR_EL1_RESET		SCTLR_EL1_RES1
> +#define HCR_EL2_RESET		HCR_EL2_RES1
>   
>   #define ID_AA64PFR0_EL1_GIC	BITS(27, 24)
>   

_______________________________________________
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] 25+ messages in thread

* Re: [bootwrapper PATCH 13/13] Unify start_el3 & start_no_el3
  2022-01-11 13:06 ` [bootwrapper PATCH 13/13] Unify start_el3 & start_no_el3 Mark Rutland
@ 2022-01-11 14:39   ` Robin Murphy
  2022-01-12 14:37     ` Mark Rutland
  0 siblings, 1 reply; 25+ messages in thread
From: Robin Murphy @ 2022-01-11 14:39 UTC (permalink / raw)
  To: Mark Rutland, linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, Wei.Chen

On 2022-01-11 13:06, Mark Rutland wrote:
> Now that the start_el3 and start_no_el3 labels point at the same place,
> unify them into a start_bootmethod label and update callers.
> 
> 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 | 5 ++---
>   arch/aarch64/boot.S | 4 ++--
>   arch/aarch64/psci.S | 3 +--
>   3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> index 820957b..4d16c9c 100644
> --- a/arch/aarch32/boot.S
> +++ b/arch/aarch32/boot.S
> @@ -51,7 +51,7 @@ ASM_FUNC(_start)
>   
>   	bl	cpu_init_bootwrapper
>   
> -	b	start_no_el3
> +	b	start_bootmethod
>   
>   _switch_monitor:
>   	adr	lr, _monitor
> @@ -69,8 +69,7 @@ _monitor:
>   
>   	bl	gic_secure_init
>   
> -	/* Initialise boot method */
> -	b	start_el3
> +	b	start_bootmethod
>   
>   err_invalid_id:
>   	b	.
> diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> index c0ec518..da5fa65 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -57,7 +57,7 @@ reset_at_el3:
>   
>   	bl	gic_secure_init
>   
> -	b	start_el3
> +	b	start_bootmethod
>   
>   	/*
>   	 * EL2 initialization
> @@ -97,7 +97,7 @@ reset_no_el3:
>   
>   	bl	cpu_init_bootwrapper
>   
> -	b	start_no_el3
> +	b	start_bootmethod
>   
>   err_invalid_id:
>   	b	.
> diff --git a/arch/aarch64/psci.S b/arch/aarch64/psci.S
> index d6ca2eb..9709dbb 100644
> --- a/arch/aarch64/psci.S
> +++ b/arch/aarch64/psci.S
> @@ -79,6 +79,5 @@ smc_exit:
>   	ldp	x18, x19, [sp], #16
>   	eret
>   
> -ASM_FUNC(start_el3)
> -ASM_FUNC(start_no_el3)
> +ASM_FUNC(start_bootmethod)

Should there be an equivalent of this hunk for arch/aarch32/psci.S?

Robin.

>   	b	psci_first_spin

_______________________________________________
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] 25+ messages in thread

* Re: [bootwrapper PATCH 02/13] Add bit-field macros
  2022-01-11 13:06 ` [bootwrapper PATCH 02/13] Add bit-field macros Mark Rutland
@ 2022-01-11 14:40   ` Andre Przywara
  2022-01-12 14:16     ` Mark Rutland
  0 siblings, 1 reply; 25+ messages in thread
From: Andre Przywara @ 2022-01-11 14:40 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Tue, 11 Jan 2022 13:06:42 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

Hi Mark,

> Arm architectural documentation typically defines bit-fields as
> `[msb,lsb]` and single-bit fields as `[bit]`. For clarity it would be
> helpful if we could define fields in the same way.
> 
> Add helpers so that we can do so, along with helper to extract/insert
> bit-field values.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  include/bits.h | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 include/bits.h
> 
> diff --git a/include/bits.h b/include/bits.h
> new file mode 100644
> index 0000000..8824a38
> --- /dev/null
> +++ b/include/bits.h
> @@ -0,0 +1,33 @@
> +/*
> + * include/bits.h - helpers for bit-field definitions.
> + *
> + * 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.
> + */
> +#ifndef __BITS_H
> +#define __BITS_H
> +
> +#ifdef __ASSEMBLY__
> +#define UL(x)	x
> +#define ULL(x)	x
> +#else
> +#define UL(x)	x##UL
> +#define ULL(x)	x##ULL
> +#endif
> +
> +#define BITS(msb, lsb) \

The kernel uses GENMASK() for this, should we follow suit here? Both
U-Boot and Trusted Firmware decided to do so, so I consider this some kind
of agreed naming for bitmask generation these days.

> +((~ULL(0) >> (63 - msb)) & (~ULL(0) << lsb))
> +
> +#define BIT(b)	BITS(b, b)
> +
> +#define BITS_LSB(bits)	(__builtin_ffsll(bits) - 1)

Shall there be some comment explaining the functionality and arguments? Or
maybe use "mask" instead of the more ambiguous "bits" name here?
TBH I needed to read the implementation of the next macro to understand
what it does.

> +
> +#define BITS_EXTRACT(val, bits) \

Same here, having BITS_EXTRACT(val, mask) looks more readable to me.

Cheers,
Andre

> +	(((val) & (bits)) >> BITS_LSB(bits))
> +
> +#define BITS_INSERT(bits, val) \
> +	(((val) << BITS_LSB(bits)) & (bits))
> +
> +#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] 25+ messages in thread

* Re: [bootwrapper PATCH 05/13] aarch64: add mov_64 macro
  2022-01-11 13:06 ` [bootwrapper PATCH 05/13] aarch64: add mov_64 macro Mark Rutland
@ 2022-01-11 14:41   ` Andre Przywara
  2022-01-12 14:18     ` Mark Rutland
  0 siblings, 1 reply; 25+ messages in thread
From: Andre Przywara @ 2022-01-11 14:41 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Tue, 11 Jan 2022 13:06:45 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

Hi,

> In subsequent patches we'll need to load 64-bit values into GPRs before
> the CPU is in a known endianness, where we cannot use literal pools.
> 
> In preparation for that, this patch adds a new `mov_64` macro to load a
> 64-bit value into a GPR using a sequence of MOV and MOVKs, which will
> function the same regardless of the CPU's endianness.
> 
> At the same time, move the `cpuid` macro to use `mov_64` internally.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/aarch64/common.S | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/aarch64/common.S b/arch/aarch64/common.S
> index c7171a9..3279fa9 100644
> --- a/arch/aarch64/common.S
> +++ b/arch/aarch64/common.S
> @@ -9,9 +9,17 @@
>  
>  #include <cpu.h>
>  
> +	/* Load a 64-bit value using immediates */
> +	.macro	mov_64 dest, val
> +	mov	\dest, #(((\val) >>  0) & 0xffff)
> +	movk	\dest, #(((\val) >> 16) & 0xffff), lsl #16
> +	movk	\dest, #(((\val) >> 32) & 0xffff), lsl #32
> +	movk	\dest, #(((\val) >> 48) & 0xffff), lsl #48
> +	.endm
> +

Trusted Firmware has an (admittedly more complicated) version that only
uses as many instructions as needed, by skipping over halfwords that are
zero:
https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/include/arch/aarch64/asm_macros.S#n125

Does that sound useful for us?

Cheers,
Andre

>  	/* Put MPIDR into \dest, clobber \tmp and flags */
>  	.macro cpuid dest, tmp
>  	mrs	\dest, mpidr_el1
> -	ldr	\tmp, =MPIDR_ID_BITS
> +	mov_64	\tmp, MPIDR_ID_BITS
>  	ands	\dest, \dest, \tmp
>  	.endm


_______________________________________________
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] 25+ messages in thread

* Re: [bootwrapper PATCH 02/13] Add bit-field macros
  2022-01-11 14:40   ` Andre Przywara
@ 2022-01-12 14:16     ` Mark Rutland
  2022-01-14 18:13       ` Andre Przywara
  0 siblings, 1 reply; 25+ messages in thread
From: Mark Rutland @ 2022-01-12 14:16 UTC (permalink / raw)
  To: Andre Przywara; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Tue, Jan 11, 2022 at 02:40:48PM +0000, Andre Przywara wrote:
> On Tue, 11 Jan 2022 13:06:42 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> Hi Mark,
> 
> > Arm architectural documentation typically defines bit-fields as
> > `[msb,lsb]` and single-bit fields as `[bit]`. For clarity it would be
> > helpful if we could define fields in the same way.
> > 
> > Add helpers so that we can do so, along with helper to extract/insert
> > bit-field values.
> > 
> > There should be no functional change as a result of this patch.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  include/bits.h | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >  create mode 100644 include/bits.h
> > 
> > diff --git a/include/bits.h b/include/bits.h
> > new file mode 100644
> > index 0000000..8824a38
> > --- /dev/null
> > +++ b/include/bits.h
> > @@ -0,0 +1,33 @@
> > +/*
> > + * include/bits.h - helpers for bit-field definitions.
> > + *
> > + * 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.
> > + */
> > +#ifndef __BITS_H
> > +#define __BITS_H
> > +
> > +#ifdef __ASSEMBLY__
> > +#define UL(x)	x
> > +#define ULL(x)	x
> > +#else
> > +#define UL(x)	x##UL
> > +#define ULL(x)	x##ULL
> > +#endif
> > +
> > +#define BITS(msb, lsb) \
> 
> The kernel uses GENMASK() for this, should we follow suit here? Both
> U-Boot and Trusted Firmware decided to do so, so I consider this some kind
> of agreed naming for bitmask generation these days.

TBH, I always forget the naming of GENMASK(), and chose `BITS()` to more
clearly align with `BIT()`, and also the way the architecture documentation
speaks about "bits [msb:lsb]".

I'm not wedded to the naming, but IMO `GENMASK()` isn't any better, even if
that's what linux uses. Regardless of the specific names, I'd like the
single-bit and multi-bit helpers to clearly align naming-wise.

For now I'd prefer to stick with `BIT()` and `BITS()`.

> > +((~ULL(0) >> (63 - msb)) & (~ULL(0) << lsb))
> > +
> > +#define BIT(b)	BITS(b, b)
> > +
> > +#define BITS_LSB(bits)	(__builtin_ffsll(bits) - 1)
> 
> Shall there be some comment explaining the functionality and arguments? Or
> maybe use "mask" instead of the more ambiguous "bits" name here?
> TBH I needed to read the implementation of the next macro to understand
> what it does.

If there's any confusion here I think we need comments regardless, since
neither `bits` nor `mask` imply contiguity, which is the important factor. I'll
add some comments with examples.

I'm happy to also rename the `bits` parameter to `mask`.

> > +
> > +#define BITS_EXTRACT(val, bits) \
> 
> Same here, having BITS_EXTRACT(val, mask) looks more readable to me.

I'll do as above hree, and likewise for the cases below.

Thanks,
Mark.

> 
> Cheers,
> Andre
> 
> > +	(((val) & (bits)) >> BITS_LSB(bits))
> > +
> > +#define BITS_INSERT(bits, val) \
> > +	(((val) << BITS_LSB(bits)) & (bits))
> > +
> > +#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] 25+ messages in thread

* Re: [bootwrapper PATCH 05/13] aarch64: add mov_64 macro
  2022-01-11 14:41   ` Andre Przywara
@ 2022-01-12 14:18     ` Mark Rutland
  2022-01-14 15:37       ` Andre Przywara
  0 siblings, 1 reply; 25+ messages in thread
From: Mark Rutland @ 2022-01-12 14:18 UTC (permalink / raw)
  To: Andre Przywara; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Tue, Jan 11, 2022 at 02:41:49PM +0000, Andre Przywara wrote:
> On Tue, 11 Jan 2022 13:06:45 +0000
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> Hi,
> 
> > In subsequent patches we'll need to load 64-bit values into GPRs before
> > the CPU is in a known endianness, where we cannot use literal pools.
> > 
> > In preparation for that, this patch adds a new `mov_64` macro to load a
> > 64-bit value into a GPR using a sequence of MOV and MOVKs, which will
> > function the same regardless of the CPU's endianness.
> > 
> > At the same time, move the `cpuid` macro to use `mov_64` internally.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> >  arch/aarch64/common.S | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/aarch64/common.S b/arch/aarch64/common.S
> > index c7171a9..3279fa9 100644
> > --- a/arch/aarch64/common.S
> > +++ b/arch/aarch64/common.S
> > @@ -9,9 +9,17 @@
> >  
> >  #include <cpu.h>
> >  
> > +	/* Load a 64-bit value using immediates */
> > +	.macro	mov_64 dest, val
> > +	mov	\dest, #(((\val) >>  0) & 0xffff)
> > +	movk	\dest, #(((\val) >> 16) & 0xffff), lsl #16
> > +	movk	\dest, #(((\val) >> 32) & 0xffff), lsl #32
> > +	movk	\dest, #(((\val) >> 48) & 0xffff), lsl #48
> > +	.endm
> > +
> 
> Trusted Firmware has an (admittedly more complicated) version that only
> uses as many instructions as needed, by skipping over halfwords that are
> zero:
> https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/include/arch/aarch64/asm_macros.S#n125
> 
> Does that sound useful for us?

For simplicity/clarity, I'd prefer to keep this as-is.

That and I'm not entirely sure about how the boot-wrapper and TF-A licenses
interact, so generally I'd strongly prefer to avoid importing code.

Thanks,
Mark.

> 
> Cheers,
> Andre
> 
> >  	/* Put MPIDR into \dest, clobber \tmp and flags */
> >  	.macro cpuid dest, tmp
> >  	mrs	\dest, mpidr_el1
> > -	ldr	\tmp, =MPIDR_ID_BITS
> > +	mov_64	\tmp, MPIDR_ID_BITS
> >  	ands	\dest, \dest, \tmp
> >  	.endm
> 

_______________________________________________
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] 25+ messages in thread

* Re: [bootwrapper PATCH 06/13] aarch64: initialize SCTLR_ELx for the boot-wrapper
  2022-01-11 14:38   ` Robin Murphy
@ 2022-01-12 14:34     ` Mark Rutland
  0 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-12 14:34 UTC (permalink / raw)
  To: Robin Murphy; +Cc: linux-arm-kernel, andre.przywara, Jaxson.Han, Wei.Chen

On Tue, Jan 11, 2022 at 02:38:28PM +0000, Robin Murphy wrote:
> On 2022-01-11 13:06, Mark Rutland wrote:
> > The SCTLR_ELx registers contain fields which are UNKNOWN or
> > IMPLEMENTATION DEFINED out of reset. This includes SCTLR_ELx.EE, which
> > defines the endianness of memory accesses (e.g. reads from literal
> > pools). Due to this, portions of boot-wrapper code are not guaranteed
> > top work correctly.
> 
> Nit: "to"

Cheers; fixed.

> > +/*
> > + * RES1 bit definitions definitions as of ARM DDI 0487G.b
> > + *
> > + * These includes bits which are RES1 in some configurations.
> > + */
> > +#define SCTLR_EL3_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
> > +				 1 << 18 | 1 << 16 | 1 << 11 | 1 << 5 | 1 << 4)
> > +
> > +#define SCTLR_EL2_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
> > +				 1 << 18 | 1 << 16 | 1 << 11 | 1 << 5 | 1 << 4)
> > +
> > +#define SCTLR_EL1_RES1		(1 << 29 | 1 << 28 | 1 << 23 | 1 << 22 | \
> > +				 1 << 11 | 1 << 8 | 1 << 7 | 1 << 4)
> > +
> > +#define HCR_EL2_RES1		(1 << 1)
> 
> Maybe use the new BIT() macro in these? I reckon that would be more readable
> overall.

Hmm; I'm not sure why I didn't do that to start with. Done.

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] 25+ messages in thread

* Re: [bootwrapper PATCH 13/13] Unify start_el3 & start_no_el3
  2022-01-11 14:39   ` Robin Murphy
@ 2022-01-12 14:37     ` Mark Rutland
  0 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-12 14:37 UTC (permalink / raw)
  To: Robin Murphy; +Cc: linux-arm-kernel, andre.przywara, Jaxson.Han, Wei.Chen

On Tue, Jan 11, 2022 at 02:39:35PM +0000, Robin Murphy wrote:
> On 2022-01-11 13:06, Mark Rutland wrote:
> > Now that the start_el3 and start_no_el3 labels point at the same place,
> > unify them into a start_bootmethod label and update callers.
> > 
> > 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 | 5 ++---
> >   arch/aarch64/boot.S | 4 ++--
> >   arch/aarch64/psci.S | 3 +--
> >   3 files changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> > index 820957b..4d16c9c 100644
> > --- a/arch/aarch32/boot.S
> > +++ b/arch/aarch32/boot.S
> > @@ -51,7 +51,7 @@ ASM_FUNC(_start)
> >   	bl	cpu_init_bootwrapper
> > -	b	start_no_el3
> > +	b	start_bootmethod
> >   _switch_monitor:
> >   	adr	lr, _monitor
> > @@ -69,8 +69,7 @@ _monitor:
> >   	bl	gic_secure_init
> > -	/* Initialise boot method */
> > -	b	start_el3
> > +	b	start_bootmethod
> >   err_invalid_id:
> >   	b	.
> > diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> > index c0ec518..da5fa65 100644
> > --- a/arch/aarch64/boot.S
> > +++ b/arch/aarch64/boot.S
> > @@ -57,7 +57,7 @@ reset_at_el3:
> >   	bl	gic_secure_init
> > -	b	start_el3
> > +	b	start_bootmethod
> >   	/*
> >   	 * EL2 initialization
> > @@ -97,7 +97,7 @@ reset_no_el3:
> >   	bl	cpu_init_bootwrapper
> > -	b	start_no_el3
> > +	b	start_bootmethod
> >   err_invalid_id:
> >   	b	.
> > diff --git a/arch/aarch64/psci.S b/arch/aarch64/psci.S
> > index d6ca2eb..9709dbb 100644
> > --- a/arch/aarch64/psci.S
> > +++ b/arch/aarch64/psci.S
> > @@ -79,6 +79,5 @@ smc_exit:
> >   	ldp	x18, x19, [sp], #16
> >   	eret
> > -ASM_FUNC(start_el3)
> > -ASM_FUNC(start_no_el3)
> > +ASM_FUNC(start_bootmethod)
> 
> Should there be an equivalent of this hunk for arch/aarch32/psci.S?

Yes, and likewise for arch/aarch64/spin.S.

I'll go fix that an re-test.

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] 25+ messages in thread

* Re: [bootwrapper PATCH 11/13] Announce locations of memory objects
  2022-01-11 13:06 ` [bootwrapper PATCH 11/13] Announce locations of memory objects Mark Rutland
@ 2022-01-14 10:48   ` Mark Rutland
  0 siblings, 0 replies; 25+ messages in thread
From: Mark Rutland @ 2022-01-14 10:48 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: andre.przywara, Jaxson.Han, Wei.Chen

On Tue, Jan 11, 2022 at 01:06:51PM +0000, Mark Rutland wrote:
> diff --git a/common/platform.c b/common/platform.c
> index 80d0562..5a91349 100644
> --- a/common/platform.c
> +++ b/common/platform.c
> @@ -52,6 +52,20 @@ void print_string(const char *str)
>  		print_char(*str++);
>  }
>  
> +#define HEX_CHARS_PER_LONG (2 * sizeof(long))
> +#define NIBBLES_PER

I've deleted this latter definition, as it was added by mistake.

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] 25+ messages in thread

* Re: [bootwrapper PATCH 05/13] aarch64: add mov_64 macro
  2022-01-12 14:18     ` Mark Rutland
@ 2022-01-14 15:37       ` Andre Przywara
  0 siblings, 0 replies; 25+ messages in thread
From: Andre Przywara @ 2022-01-14 15:37 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Wed, 12 Jan 2022 14:18:52 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

> On Tue, Jan 11, 2022 at 02:41:49PM +0000, Andre Przywara wrote:
> > On Tue, 11 Jan 2022 13:06:45 +0000
> > Mark Rutland <mark.rutland@arm.com> wrote:
> > 
> > Hi,
> >   
> > > In subsequent patches we'll need to load 64-bit values into GPRs before
> > > the CPU is in a known endianness, where we cannot use literal pools.
> > > 
> > > In preparation for that, this patch adds a new `mov_64` macro to load a
> > > 64-bit value into a GPR using a sequence of MOV and MOVKs, which will
> > > function the same regardless of the CPU's endianness.
> > > 
> > > At the same time, move the `cpuid` macro to use `mov_64` internally.
> > > 
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > ---
> > >  arch/aarch64/common.S | 10 +++++++++-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/aarch64/common.S b/arch/aarch64/common.S
> > > index c7171a9..3279fa9 100644
> > > --- a/arch/aarch64/common.S
> > > +++ b/arch/aarch64/common.S
> > > @@ -9,9 +9,17 @@
> > >  
> > >  #include <cpu.h>
> > >  
> > > +	/* Load a 64-bit value using immediates */
> > > +	.macro	mov_64 dest, val
> > > +	mov	\dest, #(((\val) >>  0) & 0xffff)
> > > +	movk	\dest, #(((\val) >> 16) & 0xffff), lsl #16
> > > +	movk	\dest, #(((\val) >> 32) & 0xffff), lsl #32
> > > +	movk	\dest, #(((\val) >> 48) & 0xffff), lsl #48
> > > +	.endm
> > > +  
> > 
> > Trusted Firmware has an (admittedly more complicated) version that only
> > uses as many instructions as needed, by skipping over halfwords that are
> > zero:
> > https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/include/arch/aarch64/asm_macros.S#n125
> > 
> > Does that sound useful for us?  
> 
> For simplicity/clarity, I'd prefer to keep this as-is.
> 
> That and I'm not entirely sure about how the boot-wrapper and TF-A licenses
> interact, so generally I'd strongly prefer to avoid importing code.

Fair enough, I just found the functionality of that TF-A version
particularly neat, though indeed somewhat hard to understand and possibly
over-engineered for us.

Cheers,
Andre

> >   
> > >  	/* Put MPIDR into \dest, clobber \tmp and flags */
> > >  	.macro cpuid dest, tmp
> > >  	mrs	\dest, mpidr_el1
> > > -	ldr	\tmp, =MPIDR_ID_BITS
> > > +	mov_64	\tmp, MPIDR_ID_BITS
> > >  	ands	\dest, \dest, \tmp
> > >  	.endm  
> >   


_______________________________________________
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] 25+ messages in thread

* Re: [bootwrapper PATCH 02/13] Add bit-field macros
  2022-01-12 14:16     ` Mark Rutland
@ 2022-01-14 18:13       ` Andre Przywara
  0 siblings, 0 replies; 25+ messages in thread
From: Andre Przywara @ 2022-01-14 18:13 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-arm-kernel, Jaxson.Han, Wei.Chen

On Wed, 12 Jan 2022 14:16:21 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

Hi,

> On Tue, Jan 11, 2022 at 02:40:48PM +0000, Andre Przywara wrote:
> > On Tue, 11 Jan 2022 13:06:42 +0000
> > Mark Rutland <mark.rutland@arm.com> wrote:
> > 
> > Hi Mark,
> >   
> > > Arm architectural documentation typically defines bit-fields as
> > > `[msb,lsb]` and single-bit fields as `[bit]`. For clarity it would be
> > > helpful if we could define fields in the same way.
> > > 
> > > Add helpers so that we can do so, along with helper to extract/insert
> > > bit-field values.
> > > 
> > > There should be no functional change as a result of this patch.
> > > 
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > ---
> > >  include/bits.h | 33 +++++++++++++++++++++++++++++++++
> > >  1 file changed, 33 insertions(+)
> > >  create mode 100644 include/bits.h
> > > 
> > > diff --git a/include/bits.h b/include/bits.h
> > > new file mode 100644
> > > index 0000000..8824a38
> > > --- /dev/null
> > > +++ b/include/bits.h
> > > @@ -0,0 +1,33 @@
> > > +/*
> > > + * include/bits.h - helpers for bit-field definitions.
> > > + *
> > > + * 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.
> > > + */
> > > +#ifndef __BITS_H
> > > +#define __BITS_H
> > > +
> > > +#ifdef __ASSEMBLY__
> > > +#define UL(x)	x
> > > +#define ULL(x)	x
> > > +#else
> > > +#define UL(x)	x##UL
> > > +#define ULL(x)	x##ULL
> > > +#endif
> > > +
> > > +#define BITS(msb, lsb) \  
> > 
> > The kernel uses GENMASK() for this, should we follow suit here? Both
> > U-Boot and Trusted Firmware decided to do so, so I consider this some kind
> > of agreed naming for bitmask generation these days.  
> 
> TBH, I always forget the naming of GENMASK(), and chose `BITS()` to more
> clearly align with `BIT()`, and also the way the architecture documentation
> speaks about "bits [msb:lsb]".
> 
> I'm not wedded to the naming, but IMO `GENMASK()` isn't any better, even if
> that's what linux uses. Regardless of the specific names, I'd like the
> single-bit and multi-bit helpers to clearly align naming-wise.
> 
> For now I'd prefer to stick with `BIT()` and `BITS()`.

Fair enough, seeing that in the code in later patches looked alright, I
guess having two arguments sets it apart enough from just BIT.

Cheers,
Andre

> > > +((~ULL(0) >> (63 - msb)) & (~ULL(0) << lsb))
> > > +
> > > +#define BIT(b)	BITS(b, b)
> > > +
> > > +#define BITS_LSB(bits)	(__builtin_ffsll(bits) - 1)  
> > 
> > Shall there be some comment explaining the functionality and arguments? Or
> > maybe use "mask" instead of the more ambiguous "bits" name here?
> > TBH I needed to read the implementation of the next macro to understand
> > what it does.  
> 
> If there's any confusion here I think we need comments regardless, since
> neither `bits` nor `mask` imply contiguity, which is the important factor. I'll
> add some comments with examples.
> 
> I'm happy to also rename the `bits` parameter to `mask`.
> 
> > > +
> > > +#define BITS_EXTRACT(val, bits) \  
> > 
> > Same here, having BITS_EXTRACT(val, mask) looks more readable to me.  
> 
> I'll do as above hree, and likewise for the cases below.
> 
> Thanks,
> Mark.
> 
> > 
> > Cheers,
> > Andre
> >   
> > > +	(((val) & (bits)) >> BITS_LSB(bits))
> > > +
> > > +#define BITS_INSERT(bits, val) \
> > > +	(((val) << BITS_LSB(bits)) & (bits))
> > > +
> > > +#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] 25+ messages in thread

end of thread, other threads:[~2022-01-14 18:14 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11 13:06 [bootwrapper PATCH 00/13] Cleanups and improvements Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 01/13] Document entry requirements Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 02/13] Add bit-field macros Mark Rutland
2022-01-11 14:40   ` Andre Przywara
2022-01-12 14:16     ` Mark Rutland
2022-01-14 18:13       ` Andre Przywara
2022-01-11 13:06 ` [bootwrapper PATCH 03/13] aarch64: add system register accessors Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 04/13] aarch32: add coprocessor accessors Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 05/13] aarch64: add mov_64 macro Mark Rutland
2022-01-11 14:41   ` Andre Przywara
2022-01-12 14:18     ` Mark Rutland
2022-01-14 15:37       ` Andre Przywara
2022-01-11 13:06 ` [bootwrapper PATCH 06/13] aarch64: initialize SCTLR_ELx for the boot-wrapper Mark Rutland
2022-01-11 14:38   ` Robin Murphy
2022-01-12 14:34     ` Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 07/13] Rework common init C code Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 08/13] Announce boot-wrapper mode / exception level Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 09/13] aarch64: move the bulk of EL3 initialization to C Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 10/13] aarch32: move the bulk of Secure PL1 " Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 11/13] Announce locations of memory objects Mark Rutland
2022-01-14 10:48   ` Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 12/13] Rework bootmethod initialization Mark Rutland
2022-01-11 13:06 ` [bootwrapper PATCH 13/13] Unify start_el3 & start_no_el3 Mark Rutland
2022-01-11 14:39   ` Robin Murphy
2022-01-12 14:37     ` Mark Rutland

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.