Re: [PATCH V4 06/10] Vlv2TbltDevicePkg/PlatformBootManager: Add capsule/recovery handling.

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Jiewen,

See feedback for QuarkPlatformPkg/PlatformBootManager.

The same feedback applies.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiewen Yao
> Sent: Saturday, October 22, 2016 7:33 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Gao, Liming <liming.gao@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>; Wei, David <david.wei@intel.com>
> Subject: [edk2] [PATCH V4 06/10] Vlv2TbltDevicePkg/PlatformBootManager: Add
> capsule/recovery handling.
> 
> 1) Add capsule and recovery boot path handling in platform BDS.
> 2) Add check if the platform is using default test key for recovery or update.
> Produce PcdTestKeyUsed to indicate if there is any
> test key used in current BIOS, such as recovery key,
> or capsule update key.
> Then the generic UI may consume this PCD to show warning information.
> 
> Cc: David Wei <david.wei@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> Reviewed-by: David Wei <david.wei@intel.com>
> ---
>  Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c      | 181 ++++++++++++++------
>  Vlv2TbltDevicePkg/Library/PlatformBdsLib/PlatformBdsLib.inf |   8 +
>  2 files changed, 134 insertions(+), 55 deletions(-)
> 
> diff --git a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
> b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
> index e1f3524..e4169b3 100644
> --- a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
> +++ b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/BdsPlatform.c
> @@ -1,15 +1,15 @@
>  /** @file
> 
>    Copyright (c) 2004  - 2016, Intel Corporation. All rights reserved.<BR>
> -
> 
> -  This program and the accompanying materials are licensed and made available under
> 
> -  the terms and conditions of the BSD License that accompanies this distribution.
> 
> -  The full text of the license may be found at
> 
> -  http://opensource.org/licenses/bsd-license.php.
> 
> -
> 
> -  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> 
> -  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> 
> -
> 
> +
> +  This program and the accompanying materials are licensed and made available under
> +  the terms and conditions of the BSD License that accompanies this distribution.
> +  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> 
> 
>  Module Name:
> @@ -45,6 +45,9 @@ Abstract:
>  #include <Library/GenericBdsLib/String.h>
>  #include <Library/NetLib.h>
> 
> +#include <Library/CapsuleLib.h>
> +#include <Protocol/EsrtManagement.h>
> +
>  EFI_GUID *ConnectDriverTable[] = {
>    &gEfiMmioDeviceProtocolGuid,
>    &gEfiI2cMasterProtocolGuid,
> @@ -1585,7 +1588,7 @@ EFIAPI
>  PlatformBdsPolicyBehavior (
>    IN OUT LIST_ENTRY                  *DriverOptionList,
>    IN OUT LIST_ENTRY                  *BootOptionList,
> -  IN PROCESS_CAPSULES                ProcessCapsules,
> +  IN PROCESS_CAPSULES                BdsProcessCapsules,
>    IN BASEM_MEMORY_TEST               BaseMemoryTest
>    )
>  {
> @@ -1594,11 +1597,8 @@ PlatformBdsPolicyBehavior (
>    EFI_BOOT_MODE                      BootMode;
>    BOOLEAN                            DeferredImageExist;
>    UINTN                              Index;
> -  CHAR16                             CapsuleVarName[36];
> -  CHAR16                             *TempVarName;
>    SYSTEM_CONFIGURATION               SystemConfiguration;
>    UINTN                              VarSize;
> -  BOOLEAN                            SetVariableFlag;
>    PLATFORM_PCI_DEVICE_PATH           *EmmcBootDevPath;
>    EFI_GLOBAL_NVS_AREA_PROTOCOL       *GlobalNvsArea;
>    EFI_HANDLE                         FvProtocolHandle;
> @@ -1612,13 +1612,14 @@ PlatformBdsPolicyBehavior (
>    BOOLEAN                            IsFirstBoot;
>    UINT16                             *BootOrder;
>    UINTN                              BootOrderSize;
> +  ESRT_MANAGEMENT_PROTOCOL           *EsrtManagement;
> 
>    Timeout = PcdGet16 (PcdPlatformBootTimeOut);
>    if (Timeout > 10 ) {
>      //we think the Timeout variable is corrupted
>      Timeout = 10;
>    }
> -
> +
>    VarSize = sizeof(SYSTEM_CONFIGURATION);
>    Status = gRT->GetVariable(
>                    NORMAL_SETUP_NAME,
> @@ -1639,7 +1640,7 @@ PlatformBdsPolicyBehavior (
>                &SystemConfiguration
>                );
>      ASSERT_EFI_ERROR (Status);
> -  }
> +  }
> 
>    //
>    // Load the driver option as the driver option list
> @@ -1652,37 +1653,6 @@ PlatformBdsPolicyBehavior (
>    BootMode = GetBootModeHob();
> 
>    //
> -  // Clear all the capsule variables CapsuleUpdateData, CapsuleUpdateData1,
> CapsuleUpdateData2...
> -  // as early as possible which will avoid the next time boot after the capsule update
> -  // will still into the capsule loop
> -  //
> -  StrCpy (CapsuleVarName, EFI_CAPSULE_VARIABLE_NAME);
> -  TempVarName = CapsuleVarName + StrLen (CapsuleVarName);
> -  Index = 0;
> -  SetVariableFlag = TRUE;
> -  while (SetVariableFlag) {
> -    if (Index > 0) {
> -      UnicodeValueToString (TempVarName, 0, Index, 0);
> -    }
> -    Status = gRT->SetVariable (
> -                    CapsuleVarName,
> -                    &gEfiCapsuleVendorGuid,
> -                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS |
> -                    EFI_VARIABLE_BOOTSERVICE_ACCESS,
> -                    0,
> -                    (VOID *)NULL
> -                    );
> -    if (EFI_ERROR (Status)) {
> -      //
> -      // There is no capsule variables, quit
> -      //
> -      SetVariableFlag = FALSE;
> -      continue;
> -    }
> -    Index++;
> -  }
> -
> -  //
>    // No deferred images exist by default
>    //
>    DeferredImageExist = FALSE;
> @@ -1733,6 +1703,11 @@ PlatformBdsPolicyBehavior (
>      }
>    }
> 
> +  Status = gBS->LocateProtocol(&gEsrtManagementProtocolGuid, NULL, (VOID
> **)&EsrtManagement);
> +  if (EFI_ERROR(Status)) {
> +    EsrtManagement = NULL;
> +  }
> +
>    switch (BootMode) {
> 
>    case BOOT_WITH_MINIMAL_CONFIGURATION:
> @@ -1822,13 +1797,18 @@ PlatformBdsPolicyBehavior (
>      #ifdef FTPM_ENABLE
>      TrEEPhysicalPresenceLibProcessRequest(NULL);
>      #endif
> +
> +    if (EsrtManagement != NULL) {
> +      EsrtManagement->LockEsrtRepository();
> +    }
> +
>      //
>      // Close boot script and install ready to lock
>      //
>      InstallReadyToLock ();
> 
>      //
> -    // Give one chance to enter the setup if we
> +    // Give one chance to enter the setup if we
>      // select Gummiboot "Reboot Into Firmware Interface" and Fast Boot is enabled.
>      //
>      BootIntoFirmwareInterface();
> @@ -1863,6 +1843,10 @@ PlatformBdsPolicyBehavior (
>        }
>      }
> 
> +    if (EsrtManagement != NULL) {
> +      EsrtManagement->LockEsrtRepository();
> +    }
> +
>      //
>      // Close boot script and install ready to lock
>      //
> @@ -1887,6 +1871,16 @@ PlatformBdsPolicyBehavior (
>      //
>      PlatformBdsConnectConsole (gPlatformConsole);
>      PlatformBdsDiagnostics (EXTENSIVE, FALSE, BaseMemoryTest);
> +
> +    DEBUG((EFI_D_INFO, "ProcessCapsules Before EndOfDxe......\n"));
> +    ProcessCapsules ();
> +    DEBUG((EFI_D_INFO, "ProcessCapsules Done\n"));
> +
> +    //
> +    // Close boot script and install ready to lock
> +    //
> +    InstallReadyToLock ();
> +
>      BdsLibConnectAll ();
> 
>      //
> @@ -1903,12 +1897,13 @@ PlatformBdsPolicyBehavior (
>        }
>      }
> 
> -    //
> -    // Close boot script and install ready to lock
> -    //
> -    InstallReadyToLock ();
> +    if (EsrtManagement != NULL) {
> +      EsrtManagement->SyncEsrtFmp();
> +    }
> 
> -    ProcessCapsules (BOOT_ON_FLASH_UPDATE);
> +    DEBUG((EFI_D_INFO, "ProcessCapsules After ConnectAll......\n"));
> +    ProcessCapsules();
> +    DEBUG((EFI_D_INFO, "ProcessCapsules Done\n"));
>      break;
> 
>    case BOOT_IN_RECOVERY_MODE:
> @@ -2012,6 +2007,10 @@ FULL_CONFIGURATION:
>     #ifdef FTPM_ENABLE
>     TrEEPhysicalPresenceLibProcessRequest(NULL);
>     #endif
> +
> +    if (EsrtManagement != NULL) {
> +      EsrtManagement->SyncEsrtFmp();
> +    }
>      //
>      // Close boot script and install ready to lock
>      //
> @@ -2029,7 +2028,7 @@ FULL_CONFIGURATION:
>      PlatformBdsEnterFrontPageWithHotKey (Timeout, FALSE);
> 
>  	//
> -	// Give one chance to enter the setup if we
> +	// Give one chance to enter the setup if we
>  	// select Gummiboot "Reboot Into Firmware Interface"
>  	//
>  	BootIntoFirmwareInterface();
> @@ -2047,7 +2046,7 @@ FULL_CONFIGURATION:
>        return;
>      }
> 
> -
> +
>      break;
>    }
> 
> @@ -2412,6 +2411,12 @@ ShowProgressHotKey (
>    EFI_GRAPHICS_OUTPUT_BLT_PIXEL Background;
>    EFI_GRAPHICS_OUTPUT_BLT_PIXEL Color;
>    UINT32                        GpioValue;
> +  CHAR16                        *TmpStr1;
> +  CHAR16                        *TmpStr2;
> +  CHAR16                        *TmpStr3;
> +  UINTN                         TmpStrSize;
> +  VOID                          *Buffer;
> +  UINTN                         Size;
> 
>    if (TimeoutDefault == 0) {
>      return EFI_TIMEOUT;
> @@ -2435,10 +2440,76 @@ ShowProgressHotKey (
>    SetMem (&Background, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0x0);
>    SetMem (&Color, sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL), 0xff);
> 
> +  TmpStr2 = NULL;
> +  TmpStr3 = NULL;
> +
> +  //
> +  // Check if the platform is using test key.
> +  //
> +  Status = GetSectionFromAnyFv(
> +             PcdGetPtr(PcdEdkiiRsa2048Sha256TestPublicKeyFileGuid),
> +             EFI_SECTION_RAW,
> +             0,
> +             &Buffer,
> +             &Size
> +             );
> +  if (!EFI_ERROR(Status)) {
> +    if ((Size == PcdGetSize(PcdRsa2048Sha256PublicKeyBuffer)) &&
> +        (CompareMem(Buffer, PcdGetPtr(PcdRsa2048Sha256PublicKeyBuffer), Size) == 0)) {
> +      TmpStr2 = L"WARNING: Recovery Test Key is used.\r\n";
> +      if (DebugAssertEnabled()) {
> +        DEBUG ((EFI_D_INFO, "\n\nWARNING: Recovery Test Key is used.\n"));
> +      } else {
> +        SerialPortWrite((UINT8 *)"\n\nWARNING: Recovery Test Key is used.",
> sizeof("\n\nWARNING: Recovery Test Key is used."));
> +      }
> +      PcdSetBoolS(PcdTestKeyUsed, TRUE);
> +    }
> +    FreePool(Buffer);
> +  }
> +  Status = GetSectionFromAnyFv(
> +             PcdGetPtr(PcdEdkiiPkcs7TestPublicKeyFileGuid),
> +             EFI_SECTION_RAW,
> +             0,
> +             &Buffer,
> +             &Size
> +             );
> +  if (!EFI_ERROR(Status)) {
> +    if ((Size == PcdGetSize(PcdPkcs7CertBuffer)) &&
> +        (CompareMem(Buffer, PcdGetPtr(PcdPkcs7CertBuffer), Size) == 0)) {
> +      TmpStr3 = L"WARNING: Capsule Test Key is used.\r\n";
> +      if (DebugAssertEnabled()) {
> +        DEBUG ((EFI_D_INFO, "\n\nWARNING: Capsule Test Key is used.\r\n"));
> +      } else {
> +        SerialPortWrite((UINT8 *)"\n\nWARNING: Capsule Test Key is used.",
> sizeof("\n\nWARNING: Capsule Test Key is used."));
> +      }
> +      PcdSetBoolS(PcdTestKeyUsed, TRUE);
> +    }
> +    FreePool(Buffer);
> +  }
> +
>    //
>    // Clear the progress status bar first
>    //
> -  TmpStr = L"Start boot option, Press <F2> or <DEL> to enter setup page.";
> +  TmpStr1 = L"Start boot option, Press <F2> or <DEL> to enter setup page.\r\n";
> +  TmpStrSize = StrSize(TmpStr1);
> +  if (TmpStr2 != NULL) {
> +    TmpStrSize += StrSize(TmpStr2);
> +  }
> +  if (TmpStr3 != NULL) {
> +    TmpStrSize += StrSize(TmpStr3);
> +  }
> +  TmpStr = AllocatePool (TmpStrSize);
> +  if (TmpStr == NULL) {
> +    TmpStr = TmpStr1;
> +  } else {
> +    StrCpyS(TmpStr, TmpStrSize/sizeof(CHAR16), TmpStr1);
> +    if (TmpStr2 != NULL) {
> +      StrCatS(TmpStr, TmpStrSize/sizeof(CHAR16), TmpStr2);
> +    }
> +    if (TmpStr3 != NULL) {
> +      StrCatS(TmpStr, TmpStrSize/sizeof(CHAR16), TmpStr3);
> +    }
> +  }
>    PlatformBdsShowProgress (Foreground, Background, TmpStr, Color, 0, 0);
> 
>    TimeoutRemain = TimeoutDefault;
> diff --git a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/PlatformBdsLib.inf
> b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/PlatformBdsLib.inf
> index c64bab9..7748e2d 100644
> --- a/Vlv2TbltDevicePkg/Library/PlatformBdsLib/PlatformBdsLib.inf
> +++ b/Vlv2TbltDevicePkg/Library/PlatformBdsLib/PlatformBdsLib.inf
> @@ -72,6 +72,7 @@
>    FileHandleLib
>    S3BootScriptLib
>    SerialPortLib
> +  CapsuleLib
> 
>  [Protocols]
>    gEfiFirmwareVolume2ProtocolGuid
> @@ -90,6 +91,7 @@
>    gEfiMmioDeviceProtocolGuid
>    gEfiI2cMasterProtocolGuid
>    gEfiI2cHostProtocolGuid
> +  gEsrtManagementProtocolGuid
> 
>  [Guids]
>    gEfiMemoryTypeInformationGuid
> @@ -119,3 +121,9 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution
>    gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdBootState
> +  gPlatformModuleTokenSpaceGuid.PcdEdkiiRsa2048Sha256TestPublicKeyFileGuid
> +  gPlatformModuleTokenSpaceGuid.PcdEdkiiPkcs7TestPublicKeyFileGuid
> +  gEfiSecurityPkgTokenSpaceGuid.PcdRsa2048Sha256PublicKeyBuffer
> +  gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed
> +
> --
> 2.7.4.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel