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 0x800001D6
>
> Reverting 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.