[edk2] ArmPlatformPkg: TC2 reset/shutdown broken in Tianocore EDK2 tree

[Ryan Harkin]

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.

Regards,
Ryan.

[Ard Biesheuvel]

Post by Ryan Harkin
The following 3 patches break reboot/shutdown from within UEFI on TC2 and
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
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.

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.

[Ryan Harkin]

Post by Ard Biesheuvel

Post by Ryan Harkin
The following 3 patches break reboot/shutdown from within UEFI on TC2 and
ee171cc 2015-05-08 ArmVExpressPkg: restrict ArmVExpressSysConfigLib to

SEC

Post by Ryan Harkin
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

Post by Ryan Harkin
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.

Post by Ard Biesheuvel
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.