From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::243; helo=mail-it0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x243.google.com (mail-it0-x243.google.com [IPv6:2607:f8b0:4001:c0b::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 196A7210FCF52 for ; Wed, 6 Jun 2018 23:57:05 -0700 (PDT) Received: by mail-it0-x243.google.com with SMTP id v83-v6so11341387itc.3 for ; Wed, 06 Jun 2018 23:57:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=0BUt6Z7aEhgZ/4OdVIEjsihZ887RZh/WvueQ0/xp+YE=; b=RYCe065+mZWTd3u+f5DSeU3qVdQvbpe4G/uBZ0oXzPSTlP7SUDn/roRbY0c18cGGXc aRrCeg4/uzA2G538MD1xo4ZgDqHVZ8kX7iXvCgZS8vULGeqesNtZLjPoG05boD4KgMO2 bd9qUpHV1jrwTpvSffyqafcYNYu4T5R15hiR4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=0BUt6Z7aEhgZ/4OdVIEjsihZ887RZh/WvueQ0/xp+YE=; b=Aw4qPaBgt0vMZpeXwfJ030Myjp2B2AKrJGteSexJ6V/+4Lt4useFsP7fAYDuxwNJAE L+BKtxzIcq1mRVg1ooFDIHfhKdTLGOuCV6JWSYyJ8byvX7kZecIPhXofGhcaLYl8XnHz 1NYt0eu4XW/XQ0rDrxZ2Tu6gbppLKiDbMIYhJ1QejR1ll4d3AZP4Wey5hgOMNIUr+M3J mh6Qga4D78jT+x1H27HoT0UAmFs84EmFUfTbNRcxhLTm3zZuTmTFfKuRIJm9jrYQ3g7L LcvsNnVqpnPTS1QgSPQJyImgmyygAp126kg+wVEUPdyfDoxDLtntweORwEN2WVWR8c8v ZV9Q== X-Gm-Message-State: APt69E3asHmufmKW0tyYUHDXoH0RNRdCup9VnX+oWwFLfRMY4OSMo3D4 bMQTTLx8LdUzeSWcEBNjz7epgwuA6TQ36QKd5UBtQylP X-Google-Smtp-Source: ADUXVKJs1Wk4n5q4gQZT8CAiM4717QcE6i3dAaPHQCPenncEms0YVslGZi3gB9FKbyZh9DaJANEQdVJARSRd2iCBMe8= X-Received: by 2002:a24:1d0e:: with SMTP id 14-v6mr808717itj.50.1528354625183; Wed, 06 Jun 2018 23:57:05 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bb86:0:0:0:0:0 with HTTP; Wed, 6 Jun 2018 23:57:04 -0700 (PDT) In-Reply-To: References: <20180606123701.4275-1-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Thu, 7 Jun 2018 08:57:04 +0200 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm 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: Thu, 07 Jun 2018 06:57:06 -0000 Content-Type: text/plain; charset="UTF-8" On 6 June 2018 at 15:29, Laszlo Ersek 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 >> --- >> 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 I will go for option #2 unless Leif has any objections to that?