[U-Boot] [PATCH] arm: socfpga: crc-protect SPL on warm boot

[Dalon L Westergreen <dalon.westergreen@linux.intel.com>]

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@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@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