* [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot @ 2018-06-06 12:37 Ard Biesheuvel 2018-06-06 12:49 ` Leif Lindholm 2018-06-06 13:29 ` Laszlo Ersek 0 siblings, 2 replies; 5+ messages in thread From: Ard Biesheuvel @ 2018-06-06 12:37 UTC (permalink / raw) To: edk2-devel; +Cc: leif.lindholm, Ard Biesheuvel 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 <ard.biesheuvel@linaro.org> --- 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); + + gBS->RaiseTPL (TPL_HIGH_LEVEL); + ArmDisableMmu (); + Reset (); + } } /** diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf index 5a1ee976e5bc..19021cd1e8b6 100644 --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf @@ -30,6 +30,15 @@ [Packages] MdePkg/MdePkg.dec [LibraryClasses] + ArmMmuLib ArmSmcLib BaseLib DebugLib + UefiBootServicesTableLib + UefiRuntimeLib + +[FeaturePcd] + gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot + +[FixedPcd] + gArmTokenSpaceGuid.PcdFvBaseAddress -- 2.17.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot 2018-06-06 12:37 [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot Ard Biesheuvel @ 2018-06-06 12:49 ` Leif Lindholm 2018-06-06 13:29 ` Laszlo Ersek 1 sibling, 0 replies; 5+ messages in thread From: Leif Lindholm @ 2018-06-06 12:49 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: edk2-devel On Wed, Jun 06, 2018 at 02:37:01PM +0200, 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 <ard.biesheuvel@linaro.org> It's not pretty, but it lets us start modelling desirable behaviour on systems we alwready have. So Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > 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); > + > + gBS->RaiseTPL (TPL_HIGH_LEVEL); > + ArmDisableMmu (); > + Reset (); > + } > } > > /** > diff --git a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf > index 5a1ee976e5bc..19021cd1e8b6 100644 > --- a/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf > +++ b/ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetSystemLib.inf > @@ -30,6 +30,15 @@ [Packages] > MdePkg/MdePkg.dec > > [LibraryClasses] > + ArmMmuLib > ArmSmcLib > BaseLib > DebugLib > + UefiBootServicesTableLib > + UefiRuntimeLib > + > +[FeaturePcd] > + gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot > + > +[FixedPcd] > + gArmTokenSpaceGuid.PcdFvBaseAddress > -- > 2.17.0 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot 2018-06-06 12:37 [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot Ard Biesheuvel 2018-06-06 12:49 ` Leif Lindholm @ 2018-06-06 13:29 ` Laszlo Ersek 2018-06-07 6:57 ` Ard Biesheuvel 1 sibling, 1 reply; 5+ messages in thread From: Laszlo Ersek @ 2018-06-06 13:29 UTC (permalink / raw) To: Ard Biesheuvel, edk2-devel; +Cc: leif.lindholm 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 <ard.biesheuvel@linaro.org> > --- > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot 2018-06-06 13:29 ` Laszlo Ersek @ 2018-06-07 6:57 ` Ard Biesheuvel 2018-06-07 14:54 ` Leif Lindholm 0 siblings, 1 reply; 5+ messages in thread From: Ard Biesheuvel @ 2018-06-07 6:57 UTC (permalink / raw) To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, Leif Lindholm On 6 June 2018 at 15:29, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org> >> --- >> 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? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot 2018-06-07 6:57 ` Ard Biesheuvel @ 2018-06-07 14:54 ` Leif Lindholm 0 siblings, 0 replies; 5+ messages in thread From: Leif Lindholm @ 2018-06-07 14:54 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: Laszlo Ersek, edk2-devel@lists.01.org On Thu, Jun 07, 2018 at 08:57:04AM +0200, Ard Biesheuvel wrote: > On 6 June 2018 at 15:29, Laszlo Ersek <lersek@redhat.com> 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 <ard.biesheuvel@linaro.org> > >> --- > >> 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? I think my preference would be the first simply becauase it feels more TianoCore. But I'm relaly happy with either option. Regards, Leif ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-07 14:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-06 12:37 [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot Ard Biesheuvel 2018-06-06 12:49 ` Leif Lindholm 2018-06-06 13:29 ` Laszlo Ersek 2018-06-07 6:57 ` Ard Biesheuvel 2018-06-07 14:54 ` Leif Lindholm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox