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

Marek Vasut marex at denx.de
Tue Nov 20 23:11:16 UTC 2018


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

-- 
Best regards,
Marek Vasut


More information about the U-Boot mailing list