[U-Boot] [PATCH v1 1/5] ARM: PSCI: initialize stack pointer on secondary CPUs

[Stefan Agner]

Hi Tom, Hi Stefano,

On 02.07.2018 11:08, Patrick DELAUNAY wrote:
> Hi Stefan
> 
>> From: Stefan Agner [mailto:stefan at agner.ch]
>> Sent: mercredi 27 juin 2018 10:36
>> Subject: Re: [PATCH v1 1/5] ARM: PSCI: initialize stack pointer on secondary CPUs
>>
>> On 24.06.2018 21:09, Stefan Agner wrote:
>> > From: Stefan Agner <stefan.agner at toradex.com>
>> >
>> > A proper stack is required to safely use C code in psci_arch_cpu_entry.
>>
>> Patrick, I prefer to have your ack on this since you introduced
>> psci_arch_cpu_entry.
>>
>> As far as I can tell STM32MP1 uses C code in psci_arch_cpu_entry. The same
>> function in i.MX 7's PSCI implementation the compiler actually pushed stuff on
>> the (uninitialized) stack, which caused the newly brought up CPU to immediately
>> crash.
>>
>> Not sure if in your case the stack pointer is already setup by some other means
>> or your compiler does not use the stack.
>>
>> In any case, I think it is better to just setup the stack properly as done in this
>> patch...
> 
> I expected that the secure stack is initialized by bootROM, but after
> check on 2018.07-rc2,
> I got a crash also with the stm32mp1 platform.
> 
> After code review, my behavior  is clearly not safe: I don't sure that
> the initial BootROM stack
> is not overlapping the installed PSCI monitor code or data.
> So I agree:  it is needed to initialize the stack in psci_cpu_entry.
> 
> Moreover after your patch the crash is solved for my platform stm32mp1.

Given that this fixes two platforms I guess it definitely should go into
v2018.07.

Technically, for i.MX 7 only patch 1, 2 and 3 are necessary, but 4 and 5
are fairly straight forward and seem to work fine here.

Patch 1 is generic, so not sure through which trees the patchset should
go...

--
Stefan

> 
>>
>> Stefano, I think we really want patch 2/3 applied before the release since it fixes
>> i.MX 7 PSCI. Right now the implementation is really broken and not PSCI 1.0
>> conformant. But patch 2/3 require this patch to be applied... Not sure how we
>> should handle this.
>>
>> --
>> Stefan
> 
> Acked-by: Patrick DELAUNAY <Patrick.delaunay at st.com>
> Tested-by: Patrick DELAUNAY <Patrick.delaunay at st.com>
> 
> Test done on stm32mp1 platform
> 
> Thanks!
> Patrick