[U-Boot] [PATCH] arm: socfpga: crc-protect SPL on warm boot
Dalon L Westergreen
dalon.westergreen at linux.intel.com
Tue Nov 20 21:55:20 UTC 2018
On Tue, 2018-11-20 at 21:54 +0100, Simon Goldschmidt wrote:
> On 20.11.2018 20:33, Marek Vasut wrote:
> > On 11/20/2018 08:22 PM, Simon Goldschmidt wrote:
> > > From: Simon Goldschmidt <sgoldschmidt at de.pepperl-fuchs.com>
> > >
> > > On socfpga gen5, a warm reboot from Linux currently triggers a warm
> > > reset via reset manager ctrl register.
is there any reason to not just promote these to cold resets?
> > >
> > > This currently leads to the boot rom just jumping to onchip ram
> > > executing the SPL that is supposed to still be there. This is
> > > because we tell the boot rom to do so by writing a magin value
> > > the warmramgrp_enable register in arch_early_init_r().
> > >
> > > However, this can lead to lockups on reboot if this register still
> > > contains its magic value but the SPL is not intact any more (e.g.
> > > partly overwritten onchip ram).
> > >
> > > To fis this, store a crc calculated over SPL in sysmgr registers to
> > > let the boot rom check it on next warm boot. If the crc is still
> > > correct, SPL can be executd from onchip ram. If the crc fails, SPL
> > > is loaded from original boot source.
> > >
> > > The crc that is written to the warmramgrp_crc register is the crc
> > > found in the SPL sfp image but with one addiional u32 added. For
> > > this, we need to add a function to calculate the updated crc. This
> > > is done as a bitwise calculation to keep the code increase small.
> > >
> > > This whole patch added 96 bytes to .text for SPL for
> > > socfpga_socrates_defconfig.
> > >
> > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> > > ---
> > >
> > > arch/arm/mach-socfpga/misc_gen5.c | 9 ----
> > > arch/arm/mach-socfpga/spl_gen5.c | 73 +++++++++++++++++++++++++++++++
> > > 2 files changed, 73 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-
> > > socfpga/misc_gen5.c
> > > index 5fa40937c4..492a3082de 100644
> > > --- a/arch/arm/mach-socfpga/misc_gen5.c
> > > +++ b/arch/arm/mach-socfpga/misc_gen5.c
> > > @@ -204,15 +204,6 @@ int arch_early_init_r(void)
> > > {
> > > int i;
> > >
> > > - /*
> > > - * Write magic value into magic register to unlock support for
> > > - * issuing warm reset. The ancient kernel code expects this
> > > - * value to be written into the register by the bootloader, so
> > > - * to support that old code, we write it here instead of in the
> > > - * reset_cpu() function just before resetting the CPU.
> > > - */
> > > - writel(0xae9efebc, &sysmgr_regs->romcodegrp_warmramgrp_enable);
> > > -
> > > for (i = 0; i < 8; i++) /* Cache initial SW setting regs
> > > */
> > > iswgrp_handoff[i] = readl(&sysmgr_regs-
> > > >iswgrp_handoff[i]);
> > >
> > > diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-
> > > socfpga/spl_gen5.c
> > > index ccdc661d05..3416e19f79 100644
> > > --- a/arch/arm/mach-socfpga/spl_gen5.c
> > > +++ b/arch/arm/mach-socfpga/spl_gen5.c
> > > @@ -63,6 +63,76 @@ u32 spl_boot_mode(const u32 boot_device)
> > > }
> > > #endif
> > >
> > > +/* This function calculates the CRC32 used by the Cyclone 5 SoC Boot Rom
> > > */
> > > +static u32 socfpga_boot_crc(u32 crc, const unsigned char *ptr, u32
> > > length)
> > > +{
> > > + uint i;
> > > + u8 bit;
> > > + unsigned char data;
> > > + const u32 poly = 0x02608edb;
> > > +
> > > + for (; length > 0; length--, ptr++) {
> > > + data = *ptr;
> > > + for (i = 0; i < 8; i++) {
> > > + if (data & 0x80)
> > > + bit = 1;
> > > + else
> > > + bit = 0;
> > > +
> > > + data = data << 1;
> > > + if (crc & 0x80000000)
> > > + bit = 1 - bit;
> > > +
> > > + if (bit) {
> > > + crc ^= poly;
> > > + crc = crc << 1;
> > > + crc |= 1;
> > > + } else {
> > > + crc = crc << 1;
> > > + }
> > > + }
> > > + }
> > > + return crc;
> > > +}
> > > +
> > > +/*
> > > + * Write magic value into magic register to unlock support for the boot
> > > rom to
> > > + * execute spl from sram on warm reset. This may be required at least on
> > > some
> > > + * boards that start from qspi where the flash chip might be in a state
> > > that
> > > + * cannot be handled by the boot rom (e.g. 4 byte mode).
> > > + *
> > > + * To prevent just jumping to corrupted memory, a crc of the spl is
> > > calculated.
> > > + * This crc is loaded from the running image, but has to be extended by
> > > the
> > > + * modified contents of the "datastart" register (i.e. 0xffff0000).
> > > + */
> > > +static void spl_init_reboot_config(void)
> > > +{
> > > + u32 spl_crc, spl_length;
> > > + const u32 spl_start = (u32)__image_copy_start;
> > > + const u32 spl_start_16 = spl_start & 0xffff;
> > > + u32 spl_length_u32;
> > > +
> > > + /* load image length from sfp header (includes crc) */
> > > + spl_length_u32 = *(const u16 *)(spl_start + 0x46);
> > > + /* subtract crc */
> > > + spl_length_u32--;
> > > + /* get length in bytes */
> > > + spl_length = spl_length_u32 * 4;
> > > + /* load crc */
> > > + spl_crc = *(const u32 *)(spl_start + spl_length);
> > > + /* undo xor */
> > > + spl_crc ^= 0xffffffff;
> > > + /* add contents of modified datastart register */
> > > + spl_crc = socfpga_boot_crc(spl_crc, (const u8 *)&spl_start, 4);
> > > + /* finalize */
> > > + spl_crc ^= 0xffffffff;
> > > +
> > > + writel(0xae9efebc, &sysmgr_regs->romcodegrp_warmramgrp_enable);
> > > + writel(spl_start_16, &sysmgr_regs->romcodegrp_warmramgrp_datastart);
> > > + writel(spl_length, &sysmgr_regs->romcodegrp_warmramgrp_length);
> > > + writel(spl_crc, &sysmgr_regs->romcodegrp_warmramgrp_crc);
> > > +}
> > > +
> > > void board_init_f(ulong dummy)
> > > {
> > > const struct cm_config *cm_default_cfg =
> > > cm_get_default_config();
> > > @@ -82,6 +152,9 @@ void board_init_f(ulong dummy)
> > > writel(SYSMGR_ECC_OCRAM_DERR | SYSMGR_ECC_OCRAM_EN,
> > > &sysmgr_regs->eccgrp_ocram);
> > >
> > > + if (!socfpga_is_booting_from_fpga())
> > > + spl_init_reboot_config();
> > > +
> > > memset(__bss_start, 0, __bss_end - __bss_start);
> > >
> > > socfpga_sdram_remap_zero();
> > >
> > Can't we use the library CRC32 function instead ?
>
> No, unfortunately, it's bit-reversed. Plus it uses a table, which
> increases the SPL binary by more than 2 KByte.
>
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
More information about the U-Boot
mailing list