On 6 June 2018 at 15:29, Laszlo Ersek <[email protected]> wrote: > On 06/06/18 14:37, Ard Biesheuvel wrote: >> Implement ResetSystemLib's EnterS3WithImmediateWake() routine using >> a jump back to the PEI entry point with interrupts and MMU+caches >> disabled. This is only possible at boot time, when we are sure that >> the current CPU is the only one up and running. Also, it depends on >> the platform whether the PEI code is preserved in memory (it may be >> copied to DRAM rather than execute in place), so also add a feature >> PCD to selectively enable this feature. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel <[email protected]> >> --- >> ArmPkg/ArmPkg.dec | 4 >> ++++ >> ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c | 21 >> ++++++++++++++++++-- >> ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf | 9 >> +++++++++ >> 3 files changed, 32 insertions(+), 2 deletions(-) >> >> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec >> index debe066b6f7b..3aa229fe2ec9 100644 >> --- a/ArmPkg/ArmPkg.dec >> +++ b/ArmPkg/ArmPkg.dec >> @@ -85,6 +85,10 @@ [PcdsFeatureFlag.common] >> # Define if the GICv3 controller should use the GICv2 legacy >> gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042 >> >> + # Whether to implement warm reboot for capsule update using a jump back >> to the >> + # PEI entry point with caches and interrupts disabled. >> + >> gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot|FALSE|BOOLEAN|0x0000001F >> + >> [PcdsFeatureFlag.ARM] >> # Whether to map normal memory as non-shareable. FALSE is the safe >> choice, but >> # TRUE may be appropriate to fix performance problems if you don't care >> about >> diff --git >> a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c >> b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c >> index d6d26bce5009..cb75e32771c2 100644 >> --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c >> +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.c >> @@ -15,10 +15,13 @@ >> >> #include <PiDxe.h> >> >> +#include <Library/ArmMmuLib.h> >> +#include <Library/ArmSmcLib.h> >> #include <Library/BaseLib.h> >> #include <Library/DebugLib.h> >> #include <Library/ResetSystemLib.h> >> -#include <Library/ArmSmcLib.h> >> +#include <Library/UefiBootServicesTableLib.h> >> +#include <Library/UefiRuntimeLib.h> >> >> #include <IndustryStandard/ArmStdSmc.h> >> >> @@ -89,7 +92,21 @@ EnterS3WithImmediateWake ( >> VOID >> ) >> { >> - // Not implemented >> + VOID (*Reset)(VOID); >> + >> + if (FeaturePcdGet (PcdArmReenterPeiForCapsuleWarmReboot) && >> + !EfiAtRuntime ()) { >> + // >> + // At boot time, we are the only core running, so we can implement the >> + // immediate wake (which is used by capsule update) by disabling the MMU >> + // and interrupts, and jumping to the PEI entry point. >> + // >> + Reset = (VOID *)(UINTN)FixedPcdGet64 (PcdFvBaseAddress); > > ISO C99 6.3.2.3 Pointers > > 1 A pointer to void may be converted to or from a pointer to any > incomplete or object type. [...] > > [...] > > 5 An integer may be converted to any pointer type. Except as > previously specified, the result is implementation-defined, might > not be correctly aligned, might not point to an entity of the > referenced type, and might be a trap representation. [...] > > [...] > > 8 A pointer to a function of one type may be converted to a pointer to > a function of another type and back again; [...] > > The point is, converting pointer-to-void to pointer-to-function is > undefined, and I expect at least clang will yell at us for it. However, > converting an integer to pointer-to-function is implementation-defined; > i.e., the C language implementation must support it to an extent, and > must document how it works. (The "previously specified" part is about > null pointer constants, in paragraph 3, which I'm not quoting now.) > > Thus, I suggest either > > typedef VOID (*RESET_FUNCTION_PTR) (VOID); > RESET_FUNCTION_PTR Reset; > > Reset = (RESET_FUNCTION_PTR)(UINTN)FixedPcdGet64 (PcdFvBaseAddress); > > or else (for kicks and giggles regarding the syntax), just > > Reset = (VOID (*)(VOID))(UINTN)FixedPcdGet64 (PcdFvBaseAddress); >
Thanks Laszlo I will go for option #2 unless Leif has any objections to that? _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
