[U-Boot] [PATCH] arm: socfpga: crc-protect SPL on warm boot
Simon Goldschmidt
simon.k.r.goldschmidt at gmail.com
Sat Nov 24 19:18:45 UTC 2018
On 21.11.2018 15:08, Marek Vasut wrote:
> On 11/21/2018 06:09 AM, Simon Goldschmidt wrote:
>>
>> Am Mi., 21. Nov. 2018, 00:11 hat Marek Vasut <marex at denx.de
>> <mailto:marex at denx.de>> geschrieben:
>>
>> On 11/20/2018 09:54 PM, 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
>> <mailto:sgoldschmidt at 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 at gmail.com
>> <mailto: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.
>>
>> Are you sure ? The uImage code also uses crc32, so I suspect that crc32
>> stuff is already in SPL. And the bit operation can probably be easily
>> done. I might be wrong ...
>>
>>
>> I wrote that as a result of testing it. The binary grew by 2k+. I'll
>> have to check the uimage crc.
> That's probably a good idea.
>
>> Bit reversing can be done, yes. I tested that too. It's a rarther small
>> function that could be added to some lib code (if it doesn't already
>> exist, I haven't checked).
> If we can reuse the CRC code, that'd be awesome.
OK, so I have v2 running with bitrev and lib/crc32. However, on the
socrates board, it does not run stable. Looks like a size issue, perhaps
some of the memory is overwritten. I'll have to investigate further.
I guess for this v1, it would be the same, depending on SPL size. So
please do not push this v1, it probably wouldn't always work.
Thanks,
Simon
More information about the U-Boot
mailing list