On 6 July 2015 at 17:32, Ard Biesheuvel ard.biesheuvel@linaro.org wrote:
On 6 July 2015 at 18:11, Ryan Harkin ryan.harkin@linaro.org wrote:
The following 3 patches break reboot/shutdown from within UEFI on TC2 and probably all 32-bit ARM Ltd platforms:
ee171cc 2015-05-08 ArmVExpressPkg: restrict ArmVExpressSysConfigLib to
SEC
and DXE_DRIVER [Ard Biesheuvel] c889bf2 2015-05-08 ArmVExpressPkg: avoid the use of ArmVExpressSysConfigLib at runtime [Ard Biesheuvel] 7b99da9 2015-05-08 ArmVExpressPkg: use PSCI for system reset at runtime [Ard Biesheuvel]
The symptoms are that the board hangs when I try to call reboot/shutdown. If I revert the top two patches only (ee171cc and c889bf2), then the
board
crashes and continuously spews out the following message:
Undefined OpCode Exception PC at 0xBF419DBC CPSR 0x800001D6Reverting all three gets reboot/shutdown working again.
Patch 7b99da9 is adding PSCI support to all ARM Ltd platforms. PSCI is a 64-bit only feature.
PSCI is definitely not a 64-bit only feature, but I agree that this breakage is bad.
No, of course it isn't. I was thinking about ARM Trusted Firmware, which is completely different and only has a 64-bit implementation.
The reason for these patches was that the system registers that are used to issue the reboot or shutdown are not (and cannot be) virtually remapped, so DXE_RUNTIME_DRIVERS should not be able to use ArmVExpressSysConfigLib. But apparently, the same DXE that is responsible for boot time reset is responsible for it at runtime.
Perhaps we should clone ArmVExpressSysConfigLib to create a version that is allowed at runtime, and takes care to only poke the sysreg interface at boot time? It wasn't feasible to add this functionality to the original, since it relies on features that the SEC phase does not provide. I am happy to propose a patch that implements the above.
I don't mind how you handle it. I don't know enough about it, only that it broke. In my tree, I've just reverted the patches.