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

[Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>]

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