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::241; helo=mail-it0-x241.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x241.google.com (mail-it0-x241.google.com [IPv6:2607:f8b0:4001:c0b::241]) (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 6744421205141 for ; Tue, 12 Jun 2018 05:26:37 -0700 (PDT) Received: by mail-it0-x241.google.com with SMTP id a3-v6so15151289itd.0 for ; Tue, 12 Jun 2018 05:26:37 -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=NHWC6ksi/rMTsC10yMoY7GFbPrETOppPPMsY6HE2N6A=; b=Xa/v9VUyelvgR20Z5stcuv+OGburAmMdDU3gOe4w+rk3zJ44HgLmwu65TvXYT6Z2BH 6cqoSyqyrd4riNvmu1vbmbiNs89fY3RI3F81QJW+mm/x1Seq/2YX7j83k+D9iafx3pmJ mjqbDKvGUyRSY411Fzn/TfniVttwntUaAlRZk= 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=NHWC6ksi/rMTsC10yMoY7GFbPrETOppPPMsY6HE2N6A=; b=RIaPmo2omxNxhXgGbXYEryepQAUWyLfso4BsDqgcI+G4NUtHGkGyO29p88bLOJKH/n NsQQ5Sk5FyNivuJnOefPe3N0HMuLsewRcDiGZyCxsNsicGFO3epoHnZaKE/dj0rdBe4d NpLaVagcVSJvlh3qshy3u5Os/WhAfVTxlvjhgGqiDe0/2BvvwkRDFlLTPduI0w8o8exy zLlD8VYsAXyBT+Epsd7ZIGpLeTV37GwE27fbC8e8sTiDljXWcSRN/VMgtzEVlPDDS2PC NNiGk8s8hOwL01LCilEa/MpP04JAxBZkZYs7hJFwGVisuhuArjc+eaiNslFjrCcJYfDy LzXQ== X-Gm-Message-State: APt69E3n8Ft6gbyV1m3SorR7ye/8vt3rN5biW7TzCUqTBpI0joJfj4DR tk7LxO+/GciXwARe65WckPPPRsf6PqBthU+OaUbSdg== X-Google-Smtp-Source: ADUXVKJlnDL774GaFu21oDicCiagoGHPn0BiVw8POtFzVBLCJy/mVOe8MRr9y/dc8j3CPu4de7WuGD1By4QyY5DWZ6I= X-Received: by 2002:a24:1d0e:: with SMTP id 14-v6mr182144itj.50.1528806396652; Tue, 12 Jun 2018 05:26:36 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Tue, 12 Jun 2018 05:26:35 -0700 (PDT) In-Reply-To: <20180612122525.qys3u5rpnlpm2ttn@bivouac.eciton.net> References: <20180612112329.664-1-ard.biesheuvel@linaro.org> <20180612112329.664-4-ard.biesheuvel@linaro.org> <20180612122525.qys3u5rpnlpm2ttn@bivouac.eciton.net> From: Ard Biesheuvel Date: Tue, 12 Jun 2018 14:26:35 +0200 Message-ID: To: Leif Lindholm Cc: "edk2-devel@lists.01.org" , "Zeng, Star" , "Yao, Jiewen" , "Kinney, Michael D" Subject: Re: [PATCH v3 3/4] ArmPkg/PlatformBootManagerLib: call ProcessCapsules() only once 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: Tue, 12 Jun 2018 12:26:37 -0000 Content-Type: text/plain; charset="UTF-8" On 12 June 2018 at 14:25, Leif Lindholm wrote: > On Tue, Jun 12, 2018 at 01:23:28PM +0200, Ard Biesheuvel wrote: >> ARM platforms have no restriction on when a system firmware update >> capsule can be applied, and so it is not necessary to call >> ProcessCapsules() twice. So let's drop the first invocation that >> occurs before EndOfDxe, and rewrite the second call so that all >> capsule updates will be applied when the console is up and able to >> provide progress feedback. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel > > I thought I acked this one last time around? Perhaps you wanted it > again after the discussion. It has been substantially modified. > Anyway: > Reviewed-by: Leif Lindholm > Thanks. >> --- >> ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 87 ++++++++++++++------ >> ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 + >> 2 files changed, 61 insertions(+), 27 deletions(-) >> >> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c >> index 3456a71fbb9c..7c21cce5960b 100644 >> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c >> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c >> @@ -24,6 +24,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -553,21 +554,6 @@ PlatformBootManagerBeforeConsole ( >> VOID >> ) >> { >> - EFI_STATUS Status; >> - ESRT_MANAGEMENT_PROTOCOL *EsrtManagement; >> - >> - if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) { >> - DEBUG ((DEBUG_INFO, "ProcessCapsules Before EndOfDxe ......\n")); >> - Status = ProcessCapsules (); >> - DEBUG ((DEBUG_INFO, "ProcessCapsules returned %r\n", Status)); >> - } else { >> - Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL, >> - (VOID **)&EsrtManagement); >> - if (!EFI_ERROR (Status)) { >> - EsrtManagement->SyncEsrtFmp (); >> - } >> - } >> - >> // >> // Signal EndOfDxe PI Event >> // >> @@ -618,6 +604,57 @@ PlatformBootManagerBeforeConsole ( >> PlatformRegisterOptionsAndKeys (); >> } >> >> +STATIC >> +VOID >> +HandleCapsules ( >> + VOID >> + ) >> +{ >> + ESRT_MANAGEMENT_PROTOCOL *EsrtManagement; >> + EFI_PEI_HOB_POINTERS HobPointer; >> + EFI_CAPSULE_HEADER *CapsuleHeader; >> + BOOLEAN NeedReset; >> + EFI_STATUS Status; >> + >> + DEBUG ((DEBUG_INFO, "%a: processing capsules ...\n", __FUNCTION__)); >> + >> + Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL, >> + (VOID **)&EsrtManagement); >> + if (!EFI_ERROR (Status)) { >> + EsrtManagement->SyncEsrtFmp (); >> + } >> + >> + // >> + // Find all capsule images from hob >> + // >> + HobPointer.Raw = GetHobList (); >> + NeedReset = FALSE; >> + while ((HobPointer.Raw = GetNextHob (EFI_HOB_TYPE_UEFI_CAPSULE, >> + HobPointer.Raw)) != NULL) { >> + CapsuleHeader = (VOID *)(UINTN)HobPointer.Capsule->BaseAddress; >> + >> + Status = ProcessCapsuleImage (CapsuleHeader); >> + if (EFI_ERROR (Status)) { >> + DEBUG ((DEBUG_ERROR, "%a: failed to process capsule %p - %r\n", >> + __FUNCTION__, CapsuleHeader, Status)); >> + return; >> + } >> + if ((CapsuleHeader->Flags & CAPSULE_FLAGS_INITIATE_RESET) != 0) { >> + NeedReset = TRUE; >> + } >> + HobPointer.Raw = GET_NEXT_HOB (HobPointer); >> + } >> + >> + if (NeedReset) { >> + DEBUG ((DEBUG_WARN, "%a: capsule update successful, resetting ...\n", >> + __FUNCTION__)); >> + >> + gRT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL); >> + CpuDeadLoop(); >> + } >> +} >> + >> + >> #define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version " >> >> /** >> @@ -637,7 +674,6 @@ PlatformBootManagerAfterConsole ( >> VOID >> ) >> { >> - ESRT_MANAGEMENT_PROTOCOL *EsrtManagement; >> EFI_STATUS Status; >> EFI_GRAPHICS_OUTPUT_PROTOCOL *GraphicsOutput; >> UINTN FirmwareVerLength; >> @@ -675,17 +711,14 @@ PlatformBootManagerAfterConsole ( >> // >> EfiBootManagerConnectAll (); >> >> - Status = gBS->LocateProtocol (&gEsrtManagementProtocolGuid, NULL, >> - (VOID **)&EsrtManagement); >> - if (!EFI_ERROR (Status)) { >> - EsrtManagement->SyncEsrtFmp (); >> - } >> - >> - if (GetBootModeHob() == BOOT_ON_FLASH_UPDATE) { >> - DEBUG((DEBUG_INFO, "ProcessCapsules After EndOfDxe ......\n")); >> - Status = ProcessCapsules (); >> - DEBUG((DEBUG_INFO, "ProcessCapsules returned %r\n", Status)); >> - } >> + // >> + // On ARM, there is currently no reason to use the phased capsule >> + // update approach where some capsules are dispatched before EndOfDxe >> + // and some are dispatched after. So just handle all capsules here, >> + // when the console is up and we can actually give the user some >> + // feedback about what is going on. >> + // >> + HandleCapsules (); >> >> // >> // Enumerate all possible boot options. >> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> index e8cbb10dabdd..28d606d5c329 100644 >> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf >> @@ -55,6 +55,7 @@ [LibraryClasses] >> UefiBootManagerLib >> UefiBootServicesTableLib >> UefiLib >> + UefiRuntimeServicesTableLib >> >> [FeaturePcd] >> gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport >> -- >> 2.17.1 >>