Message ID | 20220114105653.3003399-1-mark.rutland@arm.com (mailing list archive) |
---|---|
Headers | show |
Series | Cleanups and improvements | expand |
On Fri, 14 Jan 2022 10:56:40 +0000 Mark Rutland <mark.rutland@arm.com> wrote: Hi Mark, > 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, I intend to apply these by the end of the > day. Can you hold back with that till the beginning of next week? I got stuck halfway in the series with my review, but am on it now as we speak. Cheers, Andre > > 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. > > Since v1 [1]: > * Add comments for bit-field macros > * Use BIT() for *_RES1 definitions > * Complete start_el3/start_no_el3 cleanup > * Remove extraneous macro definition > > [1] https://lore.kernel.org/r/20220111130653.2331827-1-mark.rutland@arm.com/ > > 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 | 16 +-- > 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 | 6 +- > arch/aarch64/utils.S | 9 -- > common/boot.c | 4 - > common/init.c | 60 +++++++++++ > common/platform.c | 45 +++++--- > common/psci.c | 22 +++- > include/bits.h | 79 ++++++++++++++ > include/boot.h | 2 + > include/platform.h | 20 ++++ > model.lds.S | 20 ++-- > 25 files changed, 668 insertions(+), 230 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 >
On Fri, Jan 14, 2022 at 03:09:57PM +0000, Andre Przywara wrote: > On Fri, 14 Jan 2022 10:56:40 +0000 > Mark Rutland <mark.rutland@arm.com> wrote: > > Hi Mark, > > > If there are no objections, I intend to apply these by the end of the > > day. > > Can you hold back with that till the beginning of next week? I got stuck > halfway in the series with my review, but am on it now as we speak. Sure. Thanks for taking a look. :) Mark.