[U-Boot] [PATCH v2] arm: socfpga: do not reboot via SRAM

[Marek Vasut]

On 7/12/19 7:20 AM, Simon Goldschmidt wrote:
> On Fri, Jul 12, 2019 at 7:15 AM Marek Vasut <marex at denx.de> wrote:
>>
>> On 7/10/19 9:06 PM, Simon Goldschmidt wrote:
>>> This removes the code that enables the Boot ROM to just jump to SRAM
>>> instead of loading SPL from the original boot source on warm reboot.
>>>
>>> The reason for removing this is that it is insecure: SRAM might be
>>> overwritten at the time the warm reboot is done. Instead, use the default
>>> behaviour of loading SPL from the configured boot source medium.
>>>
>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
>>> ---
>>>
>>> Changes in v2:
>>> - remove the compatibility code restoring the old "reboot from SRAM"
>>>   behaviour via an env var callback as it turned out such a hack should
>>>   not be included by default
>>> - (v1 patch subject was: "arm: socfpga: control reboot from SRAM via env
>>>   callback")
>>>
>>>  arch/arm/mach-socfpga/misc_gen5.c | 10 +---------
>>>  1 file changed, 1 insertion(+), 9 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c
>>> index 71547d81ab..38bff8a450 100644
>>> --- a/arch/arm/mach-socfpga/misc_gen5.c
>>> +++ b/arch/arm/mach-socfpga/misc_gen5.c
>>> @@ -6,6 +6,7 @@
>>>  #include <common.h>
>>>  #include <asm/io.h>
>>>  #include <errno.h>
>>> +#include <environment.h>
>>>  #include <fdtdec.h>
>>>  #include <linux/libfdt.h>
>>>  #include <altera.h>
>>> @@ -182,15 +183,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);
>>
>> Does this break ancient altera kernel versions ? Do we care ?
> 
> You mean ancient kernels built from non-mainline sources? I don't really care,
> no.
> 
> However, this *might* break boards like my older hw revisions that set qspi into
> four byte mode. But given that, depending on the situation, those *are* already
> broken, I think removing this code is still the correct thing to do.
> 
> The bonus is that you'll notice on the very first try that 'reboot' doesn't
> work. Whereas before,t it worked at the start and then might break in some
> specific situation you'll not be able to test.

Does reboot still work in mainline Linux with this ?
I am somewhat reluctant to apply this patch, since I recall there was
some weird reason this write was added, but I don't remember what it was.

-- 
Best regards,
Marek Vasut

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-56 Email: marex at denx.de