From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C211A2112501A for ; Wed, 6 Jun 2018 06:29:34 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6E98B4005F82; Wed, 6 Jun 2018 13:29:33 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-124.rdu2.redhat.com [10.10.120.124]) by smtp.corp.redhat.com (Postfix) with ESMTP id B1BBE2026987; Wed, 6 Jun 2018 13:29:32 +0000 (UTC) To: Ard Biesheuvel , edk2-devel@lists.01.org Cc: leif.lindholm@linaro.org References: <20180606123701.4275-1-ard.biesheuvel@linaro.org> From: Laszlo Ersek Message-ID: Date: Wed, 6 Jun 2018 15:29:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180606123701.4275-1-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Wed, 06 Jun 2018 13:29:33 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Wed, 06 Jun 2018 13:29:33 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH] ArmPkg/ArmSmcPsciResetSystemLib: implement fallback for warm reboot X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jun 2018 13:29:35 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > --- > 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 > > +#include > +#include > #include > #include > #include > -#include > +#include > +#include > > #include > > @@ -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