public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom
@ 2017-11-21 10:53 kalyan-nagabhirava
  2017-11-26 15:22 ` Leif Lindholm
  0 siblings, 1 reply; 7+ messages in thread
From: kalyan-nagabhirava @ 2017-11-21 10:53 UTC (permalink / raw)
  To: edk2-devel, ard.biesheuvel
  Cc: leif.lindholm, mark.gregotski, kalyan-nagabhirava

Added required library packages related to secure boot  in hikey.dsc and Blockvariable driver[
from 96-board edk2 fork] to support the NV storage of the variables.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: kalyan-nagabhirava <kalyankumar.nagabhirava@linaro.org>
---
 Platform/Hisilicon/HiKey/HiKey.dec                 |  10 +
 Platform/Hisilicon/HiKey/HiKey.dsc                 |  56 ++-
 Platform/Hisilicon/HiKey/HiKey.fdf                 |  13 +-
 Platform/Hisilicon/HiKey/VarStore.fdf.inc          |  72 ++++
 .../Drivers/BlockVariableDxe/BlockVariableDxe.c    | 444 +++++++++++++++++++++
 .../Drivers/BlockVariableDxe/BlockVariableDxe.h    |  51 +++
 .../Drivers/BlockVariableDxe/BlockVariableDxe.inf  |  65 +++
 7 files changed, 706 insertions(+), 5 deletions(-)
 create mode 100644 Platform/Hisilicon/HiKey/VarStore.fdf.inc
 create mode 100644 Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.c
 create mode 100644 Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.h
 create mode 100644 Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf

diff --git a/Platform/Hisilicon/HiKey/HiKey.dec b/Platform/Hisilicon/HiKey/HiKey.dec
index 537138eb4..e27d70447 100644
--- a/Platform/Hisilicon/HiKey/HiKey.dec
+++ b/Platform/Hisilicon/HiKey/HiKey.dec
@@ -30,7 +30,17 @@
 
 [Guids.common]
   gHiKeyTokenSpaceGuid          =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
+  gHwTokenSpaceGuid             =  { 0x99999999, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
 
 [PcdsFixedAtBuild.common]
   gHiKeyTokenSpaceGuid.PcdAndroidFastbootNvmDevicePath|L""|VOID*|0x00000001
   gHiKeyTokenSpaceGuid.PcdArmFastbootFlashLimit|L""|VOID*|0x00000002
+
+  # NV Block
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockLba|0|UINT32|0x01000012
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockSize|0|UINT32|0x0100011
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockCount|0|UINT32|0x0100010
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockDevicePath|L""|VOID*|0x01000013
+  
+  # UncachedAllocationLib
+  gArmTokenSpaceGuid.PcdArmUncachedMemoryMask|0x0000000080000000|UINT64|0x00000002
diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc b/Platform/Hisilicon/HiKey/HiKey.dsc
index 2e3b1c879..a7288b125 100644
--- a/Platform/Hisilicon/HiKey/HiKey.dsc
+++ b/Platform/Hisilicon/HiKey/HiKey.dsc
@@ -26,6 +26,8 @@
   SKUID_IDENTIFIER               = DEFAULT
   FLASH_DEFINITION               = Platform/Hisilicon/HiKey/HiKey.fdf
 
+  DEFINE SECURE_BOOT_ENABLE	 = FALSE
+
 [LibraryClasses.common]
 !if $(TARGET) == RELEASE
   DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
@@ -125,6 +127,18 @@
   # Add support for GCC stack protector
   NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
 
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+  TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+  PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
+  BdsLib|ArmPkg/Library/BdsLib/BdsLib.inf
+  DmaLib|EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
+!endif
+
 [LibraryClasses.common.SEC]
   PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
   ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
@@ -160,6 +174,7 @@
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
 
 [BuildOptions]
   GCC:*_*_*_PLATFORM_FLAGS = -I$(WORKSPACE)/Silicon/Hisilicon/Hi6220/Include -I$(WORKSPACE)/Platform/Hisilicon/HiKey/Include
@@ -337,6 +352,29 @@
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbVendorId|0x18d1
   gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xd00d
 
+  # 
+  # NV Storage PCDs.
+  #
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockCount|0x00001000
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockSize|0x00000200
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockLba|0x00006000
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockDevicePath|L"VenHw(B549F005-4BD4-4020-A0CB-06F42BDA68C3)/HD(5,GPT,00354BCD-BBCB-4CB3-B5AE-CDEFCB5DAC43)"
+
+  #
+  # ARM Pcds
+  #
+  gArmTokenSpaceGuid.PcdArmUncachedMemoryMask|0x0000000000000000
+
+  # Increase storage space of UEFI variable to 2KB so that it can store root certificate
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x800
+
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  # override the default values from SecurityPkg to ensure images from all sources are verified in secure boot
+  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
+!endif
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
@@ -362,7 +400,6 @@
   #
   ArmPkg/Drivers/CpuDxe/CpuDxe.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
-  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
   MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
   EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
   MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
@@ -375,7 +412,6 @@
   MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
 
   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
-  MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
 
   ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
   ArmPkg/Drivers/TimerDxe/TimerDxe.inf
@@ -478,3 +514,19 @@
       gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
       gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
   }
+
+  #
+  # SecureBoot
+  #
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf  
+  Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf
+  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
+    <LibraryClasses>
+      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+  }
+!else
+  MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
+  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+!endif
diff --git a/Platform/Hisilicon/HiKey/HiKey.fdf b/Platform/Hisilicon/HiKey/HiKey.fdf
index d277bd6ce..04f98c637 100644
--- a/Platform/Hisilicon/HiKey/HiKey.fdf
+++ b/Platform/Hisilicon/HiKey/HiKey.fdf
@@ -26,12 +26,12 @@
 
 [FD.BL33_AP_UEFI]
 BaseAddress   = 0x35000000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
-Size          = 0x000F0000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
+Size          = 0x00200000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
 ErasePolarity = 1
 
 # This one is tricky, it must be: BlockSize * NumBlocks = Size
 BlockSize     = 0x00001000
-NumBlocks     = 0xF0
+NumBlocks     = 0x200
 
 ################################################################################
 #
@@ -49,10 +49,11 @@ NumBlocks     = 0xF0
 #
 ################################################################################
 
-0x00000000|0x000F0000
+0x00000000|0x00140000
 gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
 FV = FVMAIN_COMPACT
 
+!include VarStore.fdf.inc
 
 ################################################################################
 #
@@ -172,7 +173,13 @@ READ_LOCK_STATUS   = TRUE
   INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
 
   INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+  INF Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf
+!else
   INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
+!endif
 
   INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
 
diff --git a/Platform/Hisilicon/HiKey/VarStore.fdf.inc b/Platform/Hisilicon/HiKey/VarStore.fdf.inc
new file mode 100644
index 000000000..492b06abc
--- /dev/null
+++ b/Platform/Hisilicon/HiKey/VarStore.fdf.inc
@@ -0,0 +1,72 @@
+## @file
+#  FDF include file with Layout Regions that define an empty variable store.
+#
+#  Copyright (C) 2014, Red Hat, Inc.
+#  Copyright (c) 2006 - 2013, 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 which 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.
+#
+##
+
+0x00140000|0x00040000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+#NV_VARIABLE_STORE
+DATA = {
+  ## This is the EFI_FIRMWARE_VOLUME_HEADER
+  # ZeroVector []
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+  # FileSystemGuid: gEfiSystemNvDataFvGuid         =
+  #   { 0xFFF12B8D, 0x7696, 0x4C8B,
+  #     { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }}
+  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
+  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
+  # FvLength: 0xC0000
+  0x00, 0x00, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x00,
+  # Signature "_FVH"       # Attributes
+  0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
+  # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision
+  0x48, 0x00, 0x28, 0x09, 0x00, 0x00, 0x00, 0x02,
+  # Blockmap[0]: 0x3 Blocks * 0x40000 Bytes / Block
+  0x3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
+  # Blockmap[1]: End
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+  ## This is the VARIABLE_STORE_HEADER
+  # It is compatible with SECURE_BOOT_ENABLE == FALSE as well.
+  # Signature: gEfiAuthenticatedVariableGuid =
+  #   { 0xaaf32c78, 0x947b, 0x439a,
+  #     { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}
+  0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,
+  0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,
+  # Size: 0x40000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
+  #         0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8
+  # This can speed up the Variable Dispatch a bit.
+  0xB8, 0xFF, 0x03, 0x00,
+  # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
+  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+}
+
+0x00180000|0x00040000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+#NV_FTW_WORKING
+DATA = {
+  # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid         =
+  #  { 0x9e58292b, 0x7c68, 0x497d, { 0xa0, 0xce, 0x65,  0x0, 0xfd, 0x9f, 0x1b, 0x95 }}
+  0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49,
+  0xa0, 0xce, 0x65,  0x0, 0xfd, 0x9f, 0x1b, 0x95,
+  # Crc:UINT32            #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
+  0x5b, 0xe7, 0xc6, 0x86, 0xFE, 0xFF, 0xFF, 0xFF,
+  # WriteQueueSize: UINT64
+  0xE0, 0xFF, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00
+}
+
+0x001c0000|0x00040000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+#NV_FTW_SPARE
diff --git a/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.c b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.c
new file mode 100644
index 000000000..fb4c08252
--- /dev/null
+++ b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.c
@@ -0,0 +1,444 @@
+/** @file
+  This file implement the Variable Protocol for the block device.
+
+  Copyright (c) 2015, Linaro Limited. All rights reserved.
+  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which 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.
+
+**/
+
+#include <Guid/VariableFormat.h>
+#include <Guid/SystemNvDataGuid.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/CacheMaintenanceLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/IoLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+
+#include "BlockVariableDxe.h"
+
+
+STATIC EFI_PHYSICAL_ADDRESS      mMapNvStorageVariableBase;
+
+EFI_STATUS
+EFIAPI
+FvbGetAttributes (
+  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL    *This,
+  OUT       EFI_FVB_ATTRIBUTES_2                   *Attributes
+  )
+{
+  EFI_FVB_ATTRIBUTES_2 FvbAttributes;
+  FvbAttributes = (EFI_FVB_ATTRIBUTES_2) (
+
+      EFI_FVB2_READ_ENABLED_CAP | // Reads may be enabled
+      EFI_FVB2_READ_STATUS      | // Reads are currently enabled
+      EFI_FVB2_STICKY_WRITE     | // A block erase is required to flip bits into EFI_FVB2_ERASE_POLARITY
+      EFI_FVB2_MEMORY_MAPPED    | // It is memory mapped
+      EFI_FVB2_ERASE_POLARITY     // After erasure all bits take this value (i.e. '1')
+
+      );
+  FvbAttributes |= EFI_FVB2_WRITE_STATUS      | // Writes are currently enabled
+                   EFI_FVB2_WRITE_ENABLED_CAP;  // Writes may be enabled
+
+  *Attributes = FvbAttributes;
+
+  DEBUG ((DEBUG_BLKIO, "FvbGetAttributes(0x%X)\n", *Attributes));
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+FvbSetAttributes(
+  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
+  IN OUT    EFI_FVB_ATTRIBUTES_2                 *Attributes
+  )
+{
+  DEBUG ((DEBUG_BLKIO, "FvbSetAttributes(0x%X) is not supported\n",*Attributes));
+  return EFI_UNSUPPORTED;
+}
+
+EFI_STATUS
+EFIAPI
+FvbGetPhysicalAddress (
+  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
+  OUT       EFI_PHYSICAL_ADDRESS                 *Address
+  )
+{
+  *Address = mMapNvStorageVariableBase;
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+FvbGetBlockSize (
+  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
+  IN        EFI_LBA                              Lba,
+  OUT       UINTN                                *BlockSize,
+  OUT       UINTN                                *NumberOfBlocks
+  )
+{
+  BLOCK_VARIABLE_INSTANCE       *Instance;
+
+  Instance = CR (This, BLOCK_VARIABLE_INSTANCE, FvbProtocol, BLOCK_VARIABLE_SIGNATURE);
+  *BlockSize = (UINTN) Instance->Media.BlockSize;
+  *NumberOfBlocks = PcdGet32 (PcdNvStorageVariableBlockCount);
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+FvbRead (
+  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL   *This,
+  IN        EFI_LBA                               Lba,
+  IN        UINTN                                 Offset,
+  IN OUT    UINTN                                 *NumBytes,
+  IN OUT    UINT8                                 *Buffer
+  )
+{
+  BLOCK_VARIABLE_INSTANCE       *Instance;
+  EFI_BLOCK_IO_PROTOCOL         *BlockIo;
+  EFI_STATUS                    Status;
+  UINTN                         Bytes;
+  VOID                          *DataPtr;
+
+  Instance = CR (This, BLOCK_VARIABLE_INSTANCE, FvbProtocol, BLOCK_VARIABLE_SIGNATURE);
+  BlockIo = Instance->BlockIoProtocol;
+  Bytes = (Offset + *NumBytes + Instance->Media.BlockSize - 1) / Instance->Media.BlockSize * Instance->Media.BlockSize;
+  DataPtr = AllocateZeroPool (Bytes);
+  if (DataPtr == NULL) {
+    DEBUG ((EFI_D_ERROR, "FvbWrite: failed to allocate buffer.\n"));
+    return EFI_BUFFER_TOO_SMALL;
+  }
+  WriteBackDataCacheRange (DataPtr, Bytes);
+  InvalidateDataCacheRange (Buffer, *NumBytes);
+  Status = BlockIo->ReadBlocks (BlockIo, BlockIo->Media->MediaId, Instance->StartLba + Lba,
+		                Bytes, DataPtr);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "FvbRead StartLba:%x, Lba:%x, Offset:%x, Status:%x\n",
+	    Instance->StartLba, Lba, Offset, Status));
+    goto exit;
+  }
+  CopyMem (Buffer, DataPtr + Offset, *NumBytes);
+  WriteBackDataCacheRange (Buffer, *NumBytes);
+exit:
+  FreePool (DataPtr);
+  return Status;
+}
+
+EFI_STATUS
+EFIAPI
+FvbWrite (
+  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL   *This,
+  IN        EFI_LBA                               Lba,
+  IN        UINTN                                 Offset,
+  IN OUT    UINTN                                 *NumBytes,
+  IN        UINT8                                 *Buffer
+  )
+{
+  BLOCK_VARIABLE_INSTANCE       *Instance;
+  EFI_BLOCK_IO_PROTOCOL         *BlockIo;
+  EFI_STATUS                    Status;
+  UINTN                         Bytes;
+  VOID                          *DataPtr;
+
+  Instance = CR (This, BLOCK_VARIABLE_INSTANCE, FvbProtocol, BLOCK_VARIABLE_SIGNATURE);
+  BlockIo = Instance->BlockIoProtocol;
+  Bytes = (Offset + *NumBytes + Instance->Media.BlockSize - 1) / Instance->Media.BlockSize * Instance->Media.BlockSize;
+  DataPtr = AllocateZeroPool (Bytes);
+  if (DataPtr == NULL) {
+    DEBUG ((EFI_D_ERROR, "FvbWrite: failed to allocate buffer.\n"));
+    return EFI_BUFFER_TOO_SMALL;
+  }
+  WriteBackDataCacheRange (DataPtr, Bytes);
+  Status = BlockIo->ReadBlocks (BlockIo, BlockIo->Media->MediaId, Instance->StartLba + Lba,
+                                Bytes, DataPtr);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "FvbWrite: failed on reading blocks.\n"));
+    goto exit;
+  }
+  CopyMem (DataPtr + Offset, Buffer, *NumBytes);
+  WriteBackDataCacheRange (DataPtr, Bytes);
+  Status = BlockIo->WriteBlocks (BlockIo, BlockIo->Media->MediaId, Instance->StartLba + Lba,
+                                 Bytes, DataPtr);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "FvbWrite StartLba:%x, Lba:%x, Offset:%x, Status:%x\n",
+            Instance->StartLba, Lba, Offset, Status));
+  }
+  // Sometimes the variable isn't flushed into block device if it's the last flush operation.
+  // So flush it again.
+  Status = BlockIo->WriteBlocks (BlockIo, BlockIo->Media->MediaId, Instance->StartLba + Lba,
+                                 Bytes, DataPtr);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "FvbWrite StartLba:%x, Lba:%x, Offset:%x, Status:%x\n",
+            Instance->StartLba, Lba, Offset, Status));
+  }
+exit:
+  FreePool (DataPtr);
+  return Status;
+}
+
+EFI_STATUS
+EFIAPI
+FvbEraseBlocks (
+  IN CONST EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL *This,
+  ...
+  )
+{
+  return EFI_SUCCESS;
+}
+
+STATIC BLOCK_VARIABLE_INSTANCE   mBlockVariableInstance = {
+  .Signature      = BLOCK_VARIABLE_SIGNATURE,
+  .Media          = {
+    .MediaId                       = 0,
+    .RemovableMedia                = FALSE,
+    .MediaPresent                  = TRUE,
+    .LogicalPartition              = TRUE,
+    .ReadOnly                      = FALSE,
+    .WriteCaching                  = FALSE,
+    .BlockSize                     = 0,
+    .IoAlign                       = 4,
+    .LastBlock                     = 0,
+    .LowestAlignedLba              = 0,
+    .LogicalBlocksPerPhysicalBlock = 0,
+  },
+  .FvbProtocol    = {
+    .GetAttributes        = FvbGetAttributes,
+    .SetAttributes        = FvbSetAttributes,
+    .GetPhysicalAddress   = FvbGetPhysicalAddress,
+    .GetBlockSize         = FvbGetBlockSize,
+    .Read                 = FvbRead,
+    .Write                = FvbWrite,
+    .EraseBlocks          = FvbEraseBlocks,
+  }
+};
+
+EFI_STATUS
+ValidateFvHeader (
+  IN EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader
+  )
+{
+  UINT16                      Checksum, TempChecksum;
+  VARIABLE_STORE_HEADER       *VariableStoreHeader;
+  UINTN                       VariableStoreLength;
+  UINTN                       FvLength;
+
+  FvLength = (UINTN) (PcdGet32(PcdFlashNvStorageVariableSize) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) +
+      PcdGet32(PcdFlashNvStorageFtwSpareSize));
+
+  //
+  // Verify the header revision, header signature, length
+  // Length of FvBlock cannot be 2**64-1
+  // HeaderLength cannot be an odd number
+  //
+  if (   (FwVolHeader->Revision  != EFI_FVH_REVISION)
+      || (FwVolHeader->Signature != EFI_FVH_SIGNATURE)
+      || (FwVolHeader->FvLength  != FvLength)
+      )
+  {
+    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: No Firmware Volume header present\n"));
+    return EFI_NOT_FOUND;
+  }
+
+  // Check the Firmware Volume Guid
+  if( CompareGuid (&FwVolHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid) == FALSE ) {
+    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: Firmware Volume Guid non-compatible\n"));
+    return EFI_NOT_FOUND;
+  }
+
+  // Verify the header checksum
+  TempChecksum = FwVolHeader->Checksum;
+  FwVolHeader->Checksum = 0;
+  Checksum = CalculateSum16((UINT16*)FwVolHeader, FwVolHeader->HeaderLength);
+  if (Checksum != TempChecksum) {
+    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: FV checksum is invalid (Checksum:0x%X)\n",Checksum));
+    return EFI_NOT_FOUND;
+  }
+  FwVolHeader->Checksum = Checksum;
+
+  VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)FwVolHeader + FwVolHeader->HeaderLength);
+
+  // Check the Variable Store Guid
+  if( CompareGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid) == FALSE ) {
+    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: Variable Store Guid non-compatible\n"));
+    return EFI_NOT_FOUND;
+  }
+
+  VariableStoreLength = PcdGet32 (PcdFlashNvStorageVariableSize) - FwVolHeader->HeaderLength;
+  if (VariableStoreHeader->Size != VariableStoreLength) {
+    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: Variable Store Length does not match\n"));
+    return EFI_NOT_FOUND;
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+InitNonVolatileVariableStore (
+  IN BLOCK_VARIABLE_INSTANCE      *Instance,
+  IN VOID                         *Headers,
+  IN UINTN                        HeadersLength
+  )
+{
+  EFI_FIRMWARE_VOLUME_HEADER            *FirmwareVolumeHeader;
+  EFI_STATUS                            Status;
+  VARIABLE_STORE_HEADER                 *VariableStoreHeader;
+
+  // Check if the size of the area is at least one block size
+  ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->BlockIoProtocol->Media->BlockSize > 0));
+  ASSERT((PcdGet32(PcdFlashNvStorageFtwWorkingSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwWorkingSize) / Instance->BlockIoProtocol->Media->BlockSize > 0));
+  ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->BlockIoProtocol->Media->BlockSize > 0));
+
+  //
+  // EFI_FIRMWARE_VOLUME_HEADER
+  //
+  FirmwareVolumeHeader = (EFI_FIRMWARE_VOLUME_HEADER *)Headers;
+  CopyGuid (&FirmwareVolumeHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid);
+  FirmwareVolumeHeader->FvLength =
+      PcdGet32(PcdFlashNvStorageVariableSize) +
+      PcdGet32(PcdFlashNvStorageFtwWorkingSize) +
+      PcdGet32(PcdFlashNvStorageFtwSpareSize);
+  FirmwareVolumeHeader->Signature = EFI_FVH_SIGNATURE;
+  FirmwareVolumeHeader->Attributes = (EFI_FVB_ATTRIBUTES_2) (
+                                            EFI_FVB2_READ_ENABLED_CAP   | // Reads may be enabled
+                                            EFI_FVB2_READ_STATUS        | // Reads are currently enabled
+                                            EFI_FVB2_STICKY_WRITE       | // A block erase is required to flip bits into EFI_FVB2_ERASE_POLARITY
+                                            EFI_FVB2_MEMORY_MAPPED      | // It is memory mapped
+                                            EFI_FVB2_ERASE_POLARITY     | // After erasure all bits take this value (i.e. '1')
+                                            EFI_FVB2_WRITE_STATUS       | // Writes are currently enabled
+                                            EFI_FVB2_WRITE_ENABLED_CAP    // Writes may be enabled
+                                        );
+  FirmwareVolumeHeader->HeaderLength          = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY);
+  FirmwareVolumeHeader->Revision              = EFI_FVH_REVISION;
+  FirmwareVolumeHeader->BlockMap[0].NumBlocks = PcdGet32 (PcdNvStorageVariableBlockCount);
+  FirmwareVolumeHeader->BlockMap[0].Length    = Instance->BlockIoProtocol->Media->BlockSize;
+  // BlockMap Terminator
+  FirmwareVolumeHeader->BlockMap[1].NumBlocks = 0;
+  FirmwareVolumeHeader->BlockMap[1].Length    = 0;
+  FirmwareVolumeHeader->Checksum = 0;
+  FirmwareVolumeHeader->Checksum = CalculateSum16 ((UINT16*)FirmwareVolumeHeader, FirmwareVolumeHeader->HeaderLength);
+
+  //
+  // VARIABLE_STORE_HEADER
+  //
+  VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)FirmwareVolumeHeader + FirmwareVolumeHeader->HeaderLength);
+  CopyGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid);
+  VariableStoreHeader->Size = PcdGet32(PcdFlashNvStorageVariableSize) - FirmwareVolumeHeader->HeaderLength;
+  VariableStoreHeader->Format            = VARIABLE_STORE_FORMATTED;
+  VariableStoreHeader->State             = VARIABLE_STORE_HEALTHY;
+
+  Status = FvbWrite (&Instance->FvbProtocol, 0, 0, &HeadersLength, Headers);
+  return Status;
+}
+
+EFI_STATUS
+BlockVariableDxeInitialize (
+  IN EFI_HANDLE                   ImageHandle,
+  IN EFI_SYSTEM_TABLE             *SystemTable
+  )
+{
+  EFI_HANDLE                      Handle;
+  EFI_STATUS                      Status;
+  BLOCK_VARIABLE_INSTANCE         *Instance = &mBlockVariableInstance;
+  UINT32                          Count;
+  EFI_LBA                         Lba;
+  UINTN                           NvStorageSize;
+  EFI_DEVICE_PATH_PROTOCOL        *NvBlockDevicePath;
+  UINT8                           *NvStorageData;
+  VOID                            *Headers;
+  UINTN                           HeadersLength;
+
+  Instance->Signature = BLOCK_VARIABLE_SIGNATURE;
+
+  HeadersLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + sizeof(VARIABLE_STORE_HEADER);
+  Headers = AllocateZeroPool(HeadersLength);
+  if (Headers == NULL) {
+    DEBUG ((EFI_D_ERROR, "%a: failed to allocate memory of Headers\n", __func__));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  Lba = (EFI_LBA) PcdGet32 (PcdNvStorageVariableBlockLba);
+  Count = PcdGet32 (PcdNvStorageVariableBlockCount);
+  Instance->Media.BlockSize = PcdGet32 (PcdNvStorageVariableBlockSize);
+  NvStorageSize = Count * Instance->Media.BlockSize;
+  Instance->StartLba = Lba;
+  HeadersLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + sizeof(VARIABLE_STORE_HEADER);
+  if (NvStorageSize < HeadersLength) {
+    return EFI_BAD_BUFFER_SIZE;
+  }
+  NvStorageData = (UINT8 *) (UINTN) PcdGet32(PcdFlashNvStorageVariableBase);
+  mMapNvStorageVariableBase = PcdGet32(PcdFlashNvStorageVariableBase);
+  NvBlockDevicePath = &Instance->DevicePath;
+  NvBlockDevicePath = ConvertTextToDevicePath ((CHAR16*)FixedPcdGetPtr (PcdNvStorageVariableBlockDevicePath));
+  Status = gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid, &NvBlockDevicePath,
+                                  &Instance->Handle);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "Warning: Couldn't locate NVM device (status: %r)\n", Status));
+    return EFI_INVALID_PARAMETER;
+  }
+  Status = gBS->OpenProtocol (
+		      Instance->Handle,
+                      &gEfiBlockIoProtocolGuid,
+		      (VOID **) &Instance->BlockIoProtocol,
+                      gImageHandle,
+                      NULL,
+                      EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                      );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "Warning: Couldn't open NVM device (status: %r)\n", Status));
+    return EFI_DEVICE_ERROR;
+  }
+  WriteBackDataCacheRange (Instance, sizeof(BLOCK_VARIABLE_INSTANCE));
+
+  Handle = NULL;
+  Status = gBS->InstallMultipleProtocolInterfaces (
+		  &Handle,
+		  &gEfiFirmwareVolumeBlockProtocolGuid, &Instance->FvbProtocol,
+		  NULL
+		  );
+  if (EFI_ERROR (Status)) {
+    goto exit;
+  }
+
+  Status = FvbRead (&Instance->FvbProtocol, 0, 0, &HeadersLength, Headers);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = ValidateFvHeader (Headers);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "%a, Found invalid Fv Header\n", __func__));
+
+    // Erase all the block device that is reserved for variable storage
+    Status = FvbEraseBlocks (&Instance->FvbProtocol, (EFI_LBA)0, Count, EFI_LBA_LIST_TERMINATOR);
+    if (EFI_ERROR (Status)) {
+      goto exit;
+    }
+
+    Status = InitNonVolatileVariableStore (Instance, Headers, HeadersLength);
+    if (EFI_ERROR (Status)) {
+      goto exit;
+    }
+  }
+
+  if (NvStorageSize > ((EFI_FIRMWARE_VOLUME_HEADER*)Headers)->FvLength) {
+    NvStorageSize = ((EFI_FIRMWARE_VOLUME_HEADER*)Headers)->FvLength;
+    NvStorageSize = ((NvStorageSize + Instance->Media.BlockSize - 1) / Instance->Media.BlockSize) * Instance->Media.BlockSize;
+  }
+  Status = FvbRead (&Instance->FvbProtocol, 0, 0, &NvStorageSize, NvStorageData);
+
+exit:
+  return Status;
+}
diff --git a/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.h b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.h
new file mode 100644
index 000000000..a8f95e8ca
--- /dev/null
+++ b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.h
@@ -0,0 +1,51 @@
+/** @file  NorFlashDxe.h
+
+  Copyright (c) 2015, Linaro Ltd. All rights reserved.
+  Copyright (c) 2015, Hisilicon Ltd. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which 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.
+
+**/
+
+#ifndef __VARIABLE_DXE_H__
+#define __VARIABLE_DXE_H__
+
+#include <Protocol/BlockIo.h>
+#include <Protocol/DiskIo.h>
+#include <Protocol/FirmwareVolumeBlock.h>
+
+#define BLOCK_VARIABLE_SIGNATURE                       SIGNATURE_32('b', 'l', 'k', '0')
+
+typedef struct _BLOCK_VARIABLE_INSTANCE                BLOCK_VARIABLE_INSTANCE;
+
+typedef struct {
+  VENDOR_DEVICE_PATH                  Vendor;
+  EFI_DEVICE_PATH_PROTOCOL            End;
+} BLOCK_DEVICE_PATH;
+
+struct _BLOCK_VARIABLE_INSTANCE {
+  UINT32                              Signature;
+  EFI_HANDLE                          Handle;
+
+  BOOLEAN                             Initialized;
+
+  UINTN                               Size;
+  EFI_LBA                             StartLba;
+
+  EFI_BLOCK_IO_MEDIA                  Media;
+  EFI_BLOCK_IO_PROTOCOL               *BlockIoProtocol;
+  EFI_DISK_IO_PROTOCOL                DiskIoProtocol;
+  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL FvbProtocol;
+  EFI_DEVICE_PATH_PROTOCOL            DevicePath;
+
+  VOID*                               ShadowBuffer;
+};
+
+
+#endif /* __VARIABLE_DXE_H__ */
diff --git a/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf
new file mode 100644
index 000000000..9d25f3777
--- /dev/null
+++ b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf
@@ -0,0 +1,65 @@
+#/** @file
+#  INF file for the Variable Protocol implementation for the block device.
+#
+#  Copyright (c) 2015, Linaro Limited. All rights reserved.
+#  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which 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.
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = BlockVariableDxe
+  FILE_GUID                      = 522fc4a8-46d8-403e-a415-e2dbb1e0ebc0
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+
+  ENTRY_POINT                    = BlockVariableDxeInitialize
+
+[Sources.common]
+  BlockVariableDxe.c
+
+[Packages]
+  Platform/Hisilicon/HiKey/HiKey.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  ArmLib
+  BaseLib
+  BaseMemoryLib
+  CacheMaintenanceLib
+  IoLib
+  UefiDriverEntryPoint
+  UefiLib
+  DmaLib
+
+[Guids]
+  gEfiSystemNvDataFvGuid
+  gEfiVariableGuid
+  gEfiEventVirtualAddressChangeGuid
+
+[Protocols]
+  gEfiDevicePathProtocolGuid
+  gEfiBlockIoProtocolGuid			                    ## CONSUMES
+  gEfiFirmwareVolumeBlockProtocolGuid
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase      ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## CONSUMES
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockCount                  ## CONSUMES
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockSize                   ## CONSUMES
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockLba                    ## CONSUMES
+  gHwTokenSpaceGuid.PcdNvStorageVariableBlockDevicePath             ## CONSUMES
+
+[Depex]
+  TRUE
-- 
2.15.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom
  2017-11-21 10:53 [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom kalyan-nagabhirava
@ 2017-11-26 15:22 ` Leif Lindholm
  2017-11-27 13:02   ` Laszlo Ersek
  0 siblings, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2017-11-26 15:22 UTC (permalink / raw)
  To: kalyan-nagabhirava; +Cc: edk2-devel, ard.biesheuvel, mark.gregotski, lersek

(Adding Laszlo to cc based on a single comment I make below.)

On Tue, Nov 21, 2017 at 04:23:36PM +0530, kalyan-nagabhirava wrote:
> Added required library packages related to secure boot  in hikey.dsc and Blockvariable driver[
> from 96-board edk2 fork] to support the NV storage of the variables.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: kalyan-nagabhirava <kalyankumar.nagabhirava@linaro.org>
> ---
>  Platform/Hisilicon/HiKey/HiKey.dec                 |  10 +
>  Platform/Hisilicon/HiKey/HiKey.dsc                 |  56 ++-
>  Platform/Hisilicon/HiKey/HiKey.fdf                 |  13 +-
>  Platform/Hisilicon/HiKey/VarStore.fdf.inc          |  72 ++++
>  .../Drivers/BlockVariableDxe/BlockVariableDxe.c    | 444 +++++++++++++++++++++
>  .../Drivers/BlockVariableDxe/BlockVariableDxe.h    |  51 +++
>  .../Drivers/BlockVariableDxe/BlockVariableDxe.inf  |  65 +++

First high-level comments:
Like for the USB driver, could you please split this up into two
separate patches:
- one for the driver
- one for the HiKey configuration/description files.

>  7 files changed, 706 insertions(+), 5 deletions(-)
>  create mode 100644 Platform/Hisilicon/HiKey/VarStore.fdf.inc
>  create mode 100644 Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.c
>  create mode 100644 Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.h
>  create mode 100644 Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf
> 
> diff --git a/Platform/Hisilicon/HiKey/HiKey.dec b/Platform/Hisilicon/HiKey/HiKey.dec
> index 537138eb4..e27d70447 100644
> --- a/Platform/Hisilicon/HiKey/HiKey.dec
> +++ b/Platform/Hisilicon/HiKey/HiKey.dec
> @@ -30,7 +30,17 @@
>  
>  [Guids.common]
>    gHiKeyTokenSpaceGuid          =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
> +  gHwTokenSpaceGuid             =  { 0x99999999, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }

This very much looks like a not properly generated GUID.
GUIDs must always be generated using an RFC4122-compliant algorithm.
I generally recommend using
https://www.guidgenerator.com/online-guid-generator.aspx.

That said, I am not sure any of the additions to this file really
belong in this file. It feels to me these could all be added to
Silicon/Hisilicon/HisiPkg.dec, reusing gHisiTokenSpaceGuid.
(This would then belong in the first, driver, patch I mention above
rather than the second one.)

>  
>  [PcdsFixedAtBuild.common]
>    gHiKeyTokenSpaceGuid.PcdAndroidFastbootNvmDevicePath|L""|VOID*|0x00000001
>    gHiKeyTokenSpaceGuid.PcdArmFastbootFlashLimit|L""|VOID*|0x00000002
> +
> +  # NV Block
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockLba|0|UINT32|0x01000012
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockSize|0|UINT32|0x0100011
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockCount|0|UINT32|0x0100010
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockDevicePath|L""|VOID*|0x01000013
> +  
> +  # UncachedAllocationLib
> +  gArmTokenSpaceGuid.PcdArmUncachedMemoryMask|0x0000000080000000|UINT64|0x00000002

This UncachedAllocationLib stuff is not even used in this patch (but
if it was, it should not use gArmTokenSpaceGuid when declared outside
ArmPkg).

> diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc b/Platform/Hisilicon/HiKey/HiKey.dsc
> index 2e3b1c879..a7288b125 100644
> --- a/Platform/Hisilicon/HiKey/HiKey.dsc
> +++ b/Platform/Hisilicon/HiKey/HiKey.dsc
> @@ -26,6 +26,8 @@
>    SKUID_IDENTIFIER               = DEFAULT
>    FLASH_DEFINITION               = Platform/Hisilicon/HiKey/HiKey.fdf
>  
> +  DEFINE SECURE_BOOT_ENABLE	 = FALSE
> +
>  [LibraryClasses.common]
>  !if $(TARGET) == RELEASE
>    DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> @@ -125,6 +127,18 @@
>    # Add support for GCC stack protector
>    NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
>  
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> +  TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> +  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> +  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> +  PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
> +  BdsLib|ArmPkg/Library/BdsLib/BdsLib.inf
> +  DmaLib|EmbeddedPkg/Library/NonCoherentDmaLib/NonCoherentDmaLib.inf
> +!endif
> +
>  [LibraryClasses.common.SEC]
>    PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
>    ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
> @@ -160,6 +174,7 @@
>    HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>    MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
>    ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>  
>  [BuildOptions]
>    GCC:*_*_*_PLATFORM_FLAGS = -I$(WORKSPACE)/Silicon/Hisilicon/Hi6220/Include -I$(WORKSPACE)/Platform/Hisilicon/HiKey/Include
> @@ -337,6 +352,29 @@
>    gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbVendorId|0x18d1
>    gEmbeddedTokenSpaceGuid.PcdAndroidFastbootUsbProductId|0xd00d
>  
> +  # 
> +  # NV Storage PCDs.
> +  #
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockCount|0x00001000
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockSize|0x00000200
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockLba|0x00006000
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockDevicePath|L"VenHw(B549F005-4BD4-4020-A0CB-06F42BDA68C3)/HD(5,GPT,00354BCD-BBCB-4CB3-B5AE-CDEFCB5DAC43)"
> +
> +  #
> +  # ARM Pcds
> +  #
> +  gArmTokenSpaceGuid.PcdArmUncachedMemoryMask|0x0000000000000000
> +

So this one can probably be deleted too.

> +  # Increase storage space of UEFI variable to 2KB so that it can store root certificate
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x800
> +
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  # override the default values from SecurityPkg to ensure images from all sources are verified in secure boot
> +  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
> +  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
> +  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
> +!endif
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> @@ -362,7 +400,6 @@
>    #
>    ArmPkg/Drivers/CpuDxe/CpuDxe.inf
>    MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
> -  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>    MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
>    EmbeddedPkg/EmbeddedMonotonicCounter/EmbeddedMonotonicCounter.inf
>    MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
> @@ -375,7 +412,6 @@
>    MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
>  
>    MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> -  MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
>  
>    ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
>    ArmPkg/Drivers/TimerDxe/TimerDxe.inf
> @@ -478,3 +514,19 @@
>        gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
>        gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
>    }
> +
> +  #
> +  # SecureBoot
> +  #
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf  
> +  Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf
> +  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> +    <LibraryClasses>
> +      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> +  }
> +!else
> +  MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
> +  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> +!endif
> diff --git a/Platform/Hisilicon/HiKey/HiKey.fdf b/Platform/Hisilicon/HiKey/HiKey.fdf
> index d277bd6ce..04f98c637 100644
> --- a/Platform/Hisilicon/HiKey/HiKey.fdf
> +++ b/Platform/Hisilicon/HiKey/HiKey.fdf
> @@ -26,12 +26,12 @@
>  
>  [FD.BL33_AP_UEFI]
>  BaseAddress   = 0x35000000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
> -Size          = 0x000F0000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
> +Size          = 0x00200000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device

This is a substantial enough change to motivate a mention in the
commit message.

>  ErasePolarity = 1
>  
>  # This one is tricky, it must be: BlockSize * NumBlocks = Size
>  BlockSize     = 0x00001000
> -NumBlocks     = 0xF0
> +NumBlocks     = 0x200
>  
>  ################################################################################
>  #
> @@ -49,10 +49,11 @@ NumBlocks     = 0xF0
>  #
>  ################################################################################
>  
> -0x00000000|0x000F0000
> +0x00000000|0x00140000
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>  
> +!include VarStore.fdf.inc
>  
>  ################################################################################
>  #
> @@ -172,7 +173,13 @@ READ_LOCK_STATUS   = TRUE
>    INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>  
>    INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> +  INF Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf
> +!else
>    INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
> +!endif
>  
>    INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>  
> diff --git a/Platform/Hisilicon/HiKey/VarStore.fdf.inc b/Platform/Hisilicon/HiKey/VarStore.fdf.inc
> new file mode 100644
> index 000000000..492b06abc
> --- /dev/null
> +++ b/Platform/Hisilicon/HiKey/VarStore.fdf.inc
> @@ -0,0 +1,72 @@
> +## @file
> +#  FDF include file with Layout Regions that define an empty variable store.
> +#
> +#  Copyright (C) 2014, Red Hat, Inc.
> +#  Copyright (c) 2006 - 2013, 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 which 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.
> +#
> +##
> +
> +0x00140000|0x00040000
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> +#NV_VARIABLE_STORE
> +DATA = {
> +  ## This is the EFI_FIRMWARE_VOLUME_HEADER
> +  # ZeroVector []
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  # FileSystemGuid: gEfiSystemNvDataFvGuid         =
> +  #   { 0xFFF12B8D, 0x7696, 0x4C8B,
> +  #     { 0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50 }}
> +  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
> +  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
> +  # FvLength: 0xC0000
> +  0x00, 0x00, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  # Signature "_FVH"       # Attributes
> +  0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
> +  # HeaderLength # CheckSum # ExtHeaderOffset #Reserved #Revision
> +  0x48, 0x00, 0x28, 0x09, 0x00, 0x00, 0x00, 0x02,
> +  # Blockmap[0]: 0x3 Blocks * 0x40000 Bytes / Block
> +  0x3, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
> +  # Blockmap[1]: End
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  ## This is the VARIABLE_STORE_HEADER
> +  # It is compatible with SECURE_BOOT_ENABLE == FALSE as well.
> +  # Signature: gEfiAuthenticatedVariableGuid =
> +  #   { 0xaaf32c78, 0x947b, 0x439a,
> +  #     { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}
> +  0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,
> +  0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,
> +  # Size: 0x40000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) -
> +  #         0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x3ffb8
> +  # This can speed up the Variable Dispatch a bit.
> +  0xB8, 0xFF, 0x03, 0x00,
> +  # FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
> +  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +0x00180000|0x00040000
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> +#NV_FTW_WORKING
> +DATA = {
> +  # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid         =
> +  #  { 0x9e58292b, 0x7c68, 0x497d, { 0xa0, 0xce, 0x65,  0x0, 0xfd, 0x9f, 0x1b, 0x95 }}
> +  0x2b, 0x29, 0x58, 0x9e, 0x68, 0x7c, 0x7d, 0x49,
> +  0xa0, 0xce, 0x65,  0x0, 0xfd, 0x9f, 0x1b, 0x95,
> +  # Crc:UINT32            #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
> +  0x5b, 0xe7, 0xc6, 0x86, 0xFE, 0xFF, 0xFF, 0xFF,
> +  # WriteQueueSize: UINT64
> +  0xE0, 0xFF, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +0x001c0000|0x00040000
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
> +#NV_FTW_SPARE
> diff --git a/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.c b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.c
> new file mode 100644
> index 000000000..fb4c08252
> --- /dev/null
> +++ b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.c
> @@ -0,0 +1,444 @@
> +/** @file
> +  This file implement the Variable Protocol for the block device.
> +
> +  Copyright (c) 2015, Linaro Limited. All rights reserved.
> +  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which 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.
> +
> +**/
> +
> +#include <Guid/VariableFormat.h>
> +#include <Guid/SystemNvDataGuid.h>
> +
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/CacheMaintenanceLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include "BlockVariableDxe.h"
> +
> +
> +STATIC EFI_PHYSICAL_ADDRESS      mMapNvStorageVariableBase;
> +
> +EFI_STATUS
> +EFIAPI
> +FvbGetAttributes (
> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL    *This,
> +  OUT       EFI_FVB_ATTRIBUTES_2                   *Attributes
> +  )
> +{
> +  EFI_FVB_ATTRIBUTES_2 FvbAttributes;
> +  FvbAttributes = (EFI_FVB_ATTRIBUTES_2) (
> +
> +      EFI_FVB2_READ_ENABLED_CAP | // Reads may be enabled
> +      EFI_FVB2_READ_STATUS      | // Reads are currently enabled
> +      EFI_FVB2_STICKY_WRITE     | // A block erase is required to flip bits into EFI_FVB2_ERASE_POLARITY
> +      EFI_FVB2_MEMORY_MAPPED    | // It is memory mapped
> +      EFI_FVB2_ERASE_POLARITY     // After erasure all bits take this value (i.e. '1')
> +
> +      );
> +  FvbAttributes |= EFI_FVB2_WRITE_STATUS      | // Writes are currently enabled
> +                   EFI_FVB2_WRITE_ENABLED_CAP;  // Writes may be enabled
> +
> +  *Attributes = FvbAttributes;
> +
> +  DEBUG ((DEBUG_BLKIO, "FvbGetAttributes(0x%X)\n", *Attributes));
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +FvbSetAttributes(
> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
> +  IN OUT    EFI_FVB_ATTRIBUTES_2                 *Attributes
> +  )
> +{
> +  DEBUG ((DEBUG_BLKIO, "FvbSetAttributes(0x%X) is not supported\n",*Attributes));
> +  return EFI_UNSUPPORTED;

As per my (very) recent comment to Marcin, I do not believe returning
EFI_UNSUPPORTED is a valid thing to do here. Which to me suggests the
implementation of FvbGetAttributes is also incorrect.

Laszlo - what's your take on this in conjunction with PI 1.6 section
3.4.2? OvmfPkg does something very similar in
EmuVariableFvbRuntimeDxe/Fvb.c.

> +}
> +
> +EFI_STATUS
> +EFIAPI
> +FvbGetPhysicalAddress (
> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
> +  OUT       EFI_PHYSICAL_ADDRESS                 *Address
> +  )
> +{
> +  *Address = mMapNvStorageVariableBase;
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +FvbGetBlockSize (
> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
> +  IN        EFI_LBA                              Lba,
> +  OUT       UINTN                                *BlockSize,
> +  OUT       UINTN                                *NumberOfBlocks
> +  )
> +{
> +  BLOCK_VARIABLE_INSTANCE       *Instance;
> +
> +  Instance = CR (This, BLOCK_VARIABLE_INSTANCE, FvbProtocol, BLOCK_VARIABLE_SIGNATURE);
> +  *BlockSize = (UINTN) Instance->Media.BlockSize;
> +  *NumberOfBlocks = PcdGet32 (PcdNvStorageVariableBlockCount);
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +FvbRead (
> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL   *This,
> +  IN        EFI_LBA                               Lba,
> +  IN        UINTN                                 Offset,
> +  IN OUT    UINTN                                 *NumBytes,
> +  IN OUT    UINT8                                 *Buffer
> +  )
> +{
> +  BLOCK_VARIABLE_INSTANCE       *Instance;
> +  EFI_BLOCK_IO_PROTOCOL         *BlockIo;
> +  EFI_STATUS                    Status;
> +  UINTN                         Bytes;
> +  VOID                          *DataPtr;
> +
> +  Instance = CR (This, BLOCK_VARIABLE_INSTANCE, FvbProtocol, BLOCK_VARIABLE_SIGNATURE);
> +  BlockIo = Instance->BlockIoProtocol;
> +  Bytes = (Offset + *NumBytes + Instance->Media.BlockSize - 1) / Instance->Media.BlockSize * Instance->Media.BlockSize;
> +  DataPtr = AllocateZeroPool (Bytes);
> +  if (DataPtr == NULL) {
> +    DEBUG ((EFI_D_ERROR, "FvbWrite: failed to allocate buffer.\n"));
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +  WriteBackDataCacheRange (DataPtr, Bytes);
> +  InvalidateDataCacheRange (Buffer, *NumBytes);
> +  Status = BlockIo->ReadBlocks (BlockIo, BlockIo->Media->MediaId, Instance->StartLba + Lba,
> +		                Bytes, DataPtr);

Spotted some tabs in indentation here.
In general, please run patches through
"python BaseTools/Scripts/PatchCheck.py" before submission to catch
some basic formatting issues like this.

> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "FvbRead StartLba:%x, Lba:%x, Offset:%x, Status:%x\n",
> +	    Instance->StartLba, Lba, Offset, Status));

(Here too.)

/
    Leif

> +    goto exit;
> +  }
> +  CopyMem (Buffer, DataPtr + Offset, *NumBytes);
> +  WriteBackDataCacheRange (Buffer, *NumBytes);
> +exit:
> +  FreePool (DataPtr);
> +  return Status;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +FvbWrite (
> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL   *This,
> +  IN        EFI_LBA                               Lba,
> +  IN        UINTN                                 Offset,
> +  IN OUT    UINTN                                 *NumBytes,
> +  IN        UINT8                                 *Buffer
> +  )
> +{
> +  BLOCK_VARIABLE_INSTANCE       *Instance;
> +  EFI_BLOCK_IO_PROTOCOL         *BlockIo;
> +  EFI_STATUS                    Status;
> +  UINTN                         Bytes;
> +  VOID                          *DataPtr;
> +
> +  Instance = CR (This, BLOCK_VARIABLE_INSTANCE, FvbProtocol, BLOCK_VARIABLE_SIGNATURE);
> +  BlockIo = Instance->BlockIoProtocol;
> +  Bytes = (Offset + *NumBytes + Instance->Media.BlockSize - 1) / Instance->Media.BlockSize * Instance->Media.BlockSize;
> +  DataPtr = AllocateZeroPool (Bytes);
> +  if (DataPtr == NULL) {
> +    DEBUG ((EFI_D_ERROR, "FvbWrite: failed to allocate buffer.\n"));
> +    return EFI_BUFFER_TOO_SMALL;
> +  }
> +  WriteBackDataCacheRange (DataPtr, Bytes);
> +  Status = BlockIo->ReadBlocks (BlockIo, BlockIo->Media->MediaId, Instance->StartLba + Lba,
> +                                Bytes, DataPtr);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "FvbWrite: failed on reading blocks.\n"));
> +    goto exit;
> +  }
> +  CopyMem (DataPtr + Offset, Buffer, *NumBytes);
> +  WriteBackDataCacheRange (DataPtr, Bytes);
> +  Status = BlockIo->WriteBlocks (BlockIo, BlockIo->Media->MediaId, Instance->StartLba + Lba,
> +                                 Bytes, DataPtr);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "FvbWrite StartLba:%x, Lba:%x, Offset:%x, Status:%x\n",
> +            Instance->StartLba, Lba, Offset, Status));
> +  }
> +  // Sometimes the variable isn't flushed into block device if it's the last flush operation.
> +  // So flush it again.
> +  Status = BlockIo->WriteBlocks (BlockIo, BlockIo->Media->MediaId, Instance->StartLba + Lba,
> +                                 Bytes, DataPtr);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "FvbWrite StartLba:%x, Lba:%x, Offset:%x, Status:%x\n",
> +            Instance->StartLba, Lba, Offset, Status));
> +  }
> +exit:
> +  FreePool (DataPtr);
> +  return Status;
> +}
> +
> +EFI_STATUS
> +EFIAPI
> +FvbEraseBlocks (
> +  IN CONST EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL *This,
> +  ...
> +  )
> +{
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC BLOCK_VARIABLE_INSTANCE   mBlockVariableInstance = {
> +  .Signature      = BLOCK_VARIABLE_SIGNATURE,
> +  .Media          = {
> +    .MediaId                       = 0,
> +    .RemovableMedia                = FALSE,
> +    .MediaPresent                  = TRUE,
> +    .LogicalPartition              = TRUE,
> +    .ReadOnly                      = FALSE,
> +    .WriteCaching                  = FALSE,
> +    .BlockSize                     = 0,
> +    .IoAlign                       = 4,
> +    .LastBlock                     = 0,
> +    .LowestAlignedLba              = 0,
> +    .LogicalBlocksPerPhysicalBlock = 0,
> +  },
> +  .FvbProtocol    = {
> +    .GetAttributes        = FvbGetAttributes,
> +    .SetAttributes        = FvbSetAttributes,
> +    .GetPhysicalAddress   = FvbGetPhysicalAddress,
> +    .GetBlockSize         = FvbGetBlockSize,
> +    .Read                 = FvbRead,
> +    .Write                = FvbWrite,
> +    .EraseBlocks          = FvbEraseBlocks,
> +  }
> +};
> +
> +EFI_STATUS
> +ValidateFvHeader (
> +  IN EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader
> +  )
> +{
> +  UINT16                      Checksum, TempChecksum;
> +  VARIABLE_STORE_HEADER       *VariableStoreHeader;
> +  UINTN                       VariableStoreLength;
> +  UINTN                       FvLength;
> +
> +  FvLength = (UINTN) (PcdGet32(PcdFlashNvStorageVariableSize) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) +
> +      PcdGet32(PcdFlashNvStorageFtwSpareSize));
> +
> +  //
> +  // Verify the header revision, header signature, length
> +  // Length of FvBlock cannot be 2**64-1
> +  // HeaderLength cannot be an odd number
> +  //
> +  if (   (FwVolHeader->Revision  != EFI_FVH_REVISION)
> +      || (FwVolHeader->Signature != EFI_FVH_SIGNATURE)
> +      || (FwVolHeader->FvLength  != FvLength)
> +      )
> +  {
> +    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: No Firmware Volume header present\n"));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  // Check the Firmware Volume Guid
> +  if( CompareGuid (&FwVolHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid) == FALSE ) {
> +    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: Firmware Volume Guid non-compatible\n"));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  // Verify the header checksum
> +  TempChecksum = FwVolHeader->Checksum;
> +  FwVolHeader->Checksum = 0;
> +  Checksum = CalculateSum16((UINT16*)FwVolHeader, FwVolHeader->HeaderLength);
> +  if (Checksum != TempChecksum) {
> +    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: FV checksum is invalid (Checksum:0x%X)\n",Checksum));
> +    return EFI_NOT_FOUND;
> +  }
> +  FwVolHeader->Checksum = Checksum;
> +
> +  VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)FwVolHeader + FwVolHeader->HeaderLength);
> +
> +  // Check the Variable Store Guid
> +  if( CompareGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid) == FALSE ) {
> +    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: Variable Store Guid non-compatible\n"));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  VariableStoreLength = PcdGet32 (PcdFlashNvStorageVariableSize) - FwVolHeader->HeaderLength;
> +  if (VariableStoreHeader->Size != VariableStoreLength) {
> +    DEBUG ((EFI_D_ERROR, "ValidateFvHeader: Variable Store Length does not match\n"));
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +InitNonVolatileVariableStore (
> +  IN BLOCK_VARIABLE_INSTANCE      *Instance,
> +  IN VOID                         *Headers,
> +  IN UINTN                        HeadersLength
> +  )
> +{
> +  EFI_FIRMWARE_VOLUME_HEADER            *FirmwareVolumeHeader;
> +  EFI_STATUS                            Status;
> +  VARIABLE_STORE_HEADER                 *VariableStoreHeader;
> +
> +  // Check if the size of the area is at least one block size
> +  ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->BlockIoProtocol->Media->BlockSize > 0));
> +  ASSERT((PcdGet32(PcdFlashNvStorageFtwWorkingSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwWorkingSize) / Instance->BlockIoProtocol->Media->BlockSize > 0));
> +  ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->BlockIoProtocol->Media->BlockSize > 0));
> +
> +  //
> +  // EFI_FIRMWARE_VOLUME_HEADER
> +  //
> +  FirmwareVolumeHeader = (EFI_FIRMWARE_VOLUME_HEADER *)Headers;
> +  CopyGuid (&FirmwareVolumeHeader->FileSystemGuid, &gEfiSystemNvDataFvGuid);
> +  FirmwareVolumeHeader->FvLength =
> +      PcdGet32(PcdFlashNvStorageVariableSize) +
> +      PcdGet32(PcdFlashNvStorageFtwWorkingSize) +
> +      PcdGet32(PcdFlashNvStorageFtwSpareSize);
> +  FirmwareVolumeHeader->Signature = EFI_FVH_SIGNATURE;
> +  FirmwareVolumeHeader->Attributes = (EFI_FVB_ATTRIBUTES_2) (
> +                                            EFI_FVB2_READ_ENABLED_CAP   | // Reads may be enabled
> +                                            EFI_FVB2_READ_STATUS        | // Reads are currently enabled
> +                                            EFI_FVB2_STICKY_WRITE       | // A block erase is required to flip bits into EFI_FVB2_ERASE_POLARITY
> +                                            EFI_FVB2_MEMORY_MAPPED      | // It is memory mapped
> +                                            EFI_FVB2_ERASE_POLARITY     | // After erasure all bits take this value (i.e. '1')
> +                                            EFI_FVB2_WRITE_STATUS       | // Writes are currently enabled
> +                                            EFI_FVB2_WRITE_ENABLED_CAP    // Writes may be enabled
> +                                        );
> +  FirmwareVolumeHeader->HeaderLength          = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY);
> +  FirmwareVolumeHeader->Revision              = EFI_FVH_REVISION;
> +  FirmwareVolumeHeader->BlockMap[0].NumBlocks = PcdGet32 (PcdNvStorageVariableBlockCount);
> +  FirmwareVolumeHeader->BlockMap[0].Length    = Instance->BlockIoProtocol->Media->BlockSize;
> +  // BlockMap Terminator
> +  FirmwareVolumeHeader->BlockMap[1].NumBlocks = 0;
> +  FirmwareVolumeHeader->BlockMap[1].Length    = 0;
> +  FirmwareVolumeHeader->Checksum = 0;
> +  FirmwareVolumeHeader->Checksum = CalculateSum16 ((UINT16*)FirmwareVolumeHeader, FirmwareVolumeHeader->HeaderLength);
> +
> +  //
> +  // VARIABLE_STORE_HEADER
> +  //
> +  VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)FirmwareVolumeHeader + FirmwareVolumeHeader->HeaderLength);
> +  CopyGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid);
> +  VariableStoreHeader->Size = PcdGet32(PcdFlashNvStorageVariableSize) - FirmwareVolumeHeader->HeaderLength;
> +  VariableStoreHeader->Format            = VARIABLE_STORE_FORMATTED;
> +  VariableStoreHeader->State             = VARIABLE_STORE_HEALTHY;
> +
> +  Status = FvbWrite (&Instance->FvbProtocol, 0, 0, &HeadersLength, Headers);
> +  return Status;
> +}
> +
> +EFI_STATUS
> +BlockVariableDxeInitialize (
> +  IN EFI_HANDLE                   ImageHandle,
> +  IN EFI_SYSTEM_TABLE             *SystemTable
> +  )
> +{
> +  EFI_HANDLE                      Handle;
> +  EFI_STATUS                      Status;
> +  BLOCK_VARIABLE_INSTANCE         *Instance = &mBlockVariableInstance;
> +  UINT32                          Count;
> +  EFI_LBA                         Lba;
> +  UINTN                           NvStorageSize;
> +  EFI_DEVICE_PATH_PROTOCOL        *NvBlockDevicePath;
> +  UINT8                           *NvStorageData;
> +  VOID                            *Headers;
> +  UINTN                           HeadersLength;
> +
> +  Instance->Signature = BLOCK_VARIABLE_SIGNATURE;
> +
> +  HeadersLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + sizeof(VARIABLE_STORE_HEADER);
> +  Headers = AllocateZeroPool(HeadersLength);
> +  if (Headers == NULL) {
> +    DEBUG ((EFI_D_ERROR, "%a: failed to allocate memory of Headers\n", __func__));
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  Lba = (EFI_LBA) PcdGet32 (PcdNvStorageVariableBlockLba);
> +  Count = PcdGet32 (PcdNvStorageVariableBlockCount);
> +  Instance->Media.BlockSize = PcdGet32 (PcdNvStorageVariableBlockSize);
> +  NvStorageSize = Count * Instance->Media.BlockSize;
> +  Instance->StartLba = Lba;
> +  HeadersLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + sizeof(VARIABLE_STORE_HEADER);
> +  if (NvStorageSize < HeadersLength) {
> +    return EFI_BAD_BUFFER_SIZE;
> +  }
> +  NvStorageData = (UINT8 *) (UINTN) PcdGet32(PcdFlashNvStorageVariableBase);
> +  mMapNvStorageVariableBase = PcdGet32(PcdFlashNvStorageVariableBase);
> +  NvBlockDevicePath = &Instance->DevicePath;
> +  NvBlockDevicePath = ConvertTextToDevicePath ((CHAR16*)FixedPcdGetPtr (PcdNvStorageVariableBlockDevicePath));
> +  Status = gBS->LocateDevicePath (&gEfiBlockIoProtocolGuid, &NvBlockDevicePath,
> +                                  &Instance->Handle);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "Warning: Couldn't locate NVM device (status: %r)\n", Status));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  Status = gBS->OpenProtocol (
> +		      Instance->Handle,
> +                      &gEfiBlockIoProtocolGuid,
> +		      (VOID **) &Instance->BlockIoProtocol,
> +                      gImageHandle,
> +                      NULL,
> +                      EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +                      );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "Warning: Couldn't open NVM device (status: %r)\n", Status));
> +    return EFI_DEVICE_ERROR;
> +  }
> +  WriteBackDataCacheRange (Instance, sizeof(BLOCK_VARIABLE_INSTANCE));
> +
> +  Handle = NULL;
> +  Status = gBS->InstallMultipleProtocolInterfaces (
> +		  &Handle,
> +		  &gEfiFirmwareVolumeBlockProtocolGuid, &Instance->FvbProtocol,
> +		  NULL
> +		  );
> +  if (EFI_ERROR (Status)) {
> +    goto exit;
> +  }
> +
> +  Status = FvbRead (&Instance->FvbProtocol, 0, 0, &HeadersLength, Headers);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = ValidateFvHeader (Headers);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_ERROR, "%a, Found invalid Fv Header\n", __func__));
> +
> +    // Erase all the block device that is reserved for variable storage
> +    Status = FvbEraseBlocks (&Instance->FvbProtocol, (EFI_LBA)0, Count, EFI_LBA_LIST_TERMINATOR);
> +    if (EFI_ERROR (Status)) {
> +      goto exit;
> +    }
> +
> +    Status = InitNonVolatileVariableStore (Instance, Headers, HeadersLength);
> +    if (EFI_ERROR (Status)) {
> +      goto exit;
> +    }
> +  }
> +
> +  if (NvStorageSize > ((EFI_FIRMWARE_VOLUME_HEADER*)Headers)->FvLength) {
> +    NvStorageSize = ((EFI_FIRMWARE_VOLUME_HEADER*)Headers)->FvLength;
> +    NvStorageSize = ((NvStorageSize + Instance->Media.BlockSize - 1) / Instance->Media.BlockSize) * Instance->Media.BlockSize;
> +  }
> +  Status = FvbRead (&Instance->FvbProtocol, 0, 0, &NvStorageSize, NvStorageData);
> +
> +exit:
> +  return Status;
> +}
> diff --git a/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.h b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.h
> new file mode 100644
> index 000000000..a8f95e8ca
> --- /dev/null
> +++ b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.h
> @@ -0,0 +1,51 @@
> +/** @file  NorFlashDxe.h
> +
> +  Copyright (c) 2015, Linaro Ltd. All rights reserved.
> +  Copyright (c) 2015, Hisilicon Ltd. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which 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.
> +
> +**/
> +
> +#ifndef __VARIABLE_DXE_H__
> +#define __VARIABLE_DXE_H__
> +
> +#include <Protocol/BlockIo.h>
> +#include <Protocol/DiskIo.h>
> +#include <Protocol/FirmwareVolumeBlock.h>
> +
> +#define BLOCK_VARIABLE_SIGNATURE                       SIGNATURE_32('b', 'l', 'k', '0')
> +
> +typedef struct _BLOCK_VARIABLE_INSTANCE                BLOCK_VARIABLE_INSTANCE;
> +
> +typedef struct {
> +  VENDOR_DEVICE_PATH                  Vendor;
> +  EFI_DEVICE_PATH_PROTOCOL            End;
> +} BLOCK_DEVICE_PATH;
> +
> +struct _BLOCK_VARIABLE_INSTANCE {
> +  UINT32                              Signature;
> +  EFI_HANDLE                          Handle;
> +
> +  BOOLEAN                             Initialized;
> +
> +  UINTN                               Size;
> +  EFI_LBA                             StartLba;
> +
> +  EFI_BLOCK_IO_MEDIA                  Media;
> +  EFI_BLOCK_IO_PROTOCOL               *BlockIoProtocol;
> +  EFI_DISK_IO_PROTOCOL                DiskIoProtocol;
> +  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL FvbProtocol;
> +  EFI_DEVICE_PATH_PROTOCOL            DevicePath;
> +
> +  VOID*                               ShadowBuffer;
> +};
> +
> +
> +#endif /* __VARIABLE_DXE_H__ */
> diff --git a/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf
> new file mode 100644
> index 000000000..9d25f3777
> --- /dev/null
> +++ b/Silicon/Hisilicon/Drivers/BlockVariableDxe/BlockVariableDxe.inf
> @@ -0,0 +1,65 @@
> +#/** @file
> +#  INF file for the Variable Protocol implementation for the block device.
> +#
> +#  Copyright (c) 2015, Linaro Limited. All rights reserved.
> +#  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which 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.
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = BlockVariableDxe
> +  FILE_GUID                      = 522fc4a8-46d8-403e-a415-e2dbb1e0ebc0
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +
> +  ENTRY_POINT                    = BlockVariableDxeInitialize
> +
> +[Sources.common]
> +  BlockVariableDxe.c
> +
> +[Packages]
> +  Platform/Hisilicon/HiKey/HiKey.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> +  ArmLib
> +  BaseLib
> +  BaseMemoryLib
> +  CacheMaintenanceLib
> +  IoLib
> +  UefiDriverEntryPoint
> +  UefiLib
> +  DmaLib
> +
> +[Guids]
> +  gEfiSystemNvDataFvGuid
> +  gEfiVariableGuid
> +  gEfiEventVirtualAddressChangeGuid
> +
> +[Protocols]
> +  gEfiDevicePathProtocolGuid
> +  gEfiBlockIoProtocolGuid			                    ## CONSUMES
> +  gEfiFirmwareVolumeBlockProtocolGuid
> +
> +[Pcd]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase      ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize      ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize    ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize      ## CONSUMES
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockCount                  ## CONSUMES
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockSize                   ## CONSUMES
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockLba                    ## CONSUMES
> +  gHwTokenSpaceGuid.PcdNvStorageVariableBlockDevicePath             ## CONSUMES
> +
> +[Depex]
> +  TRUE
> -- 
> 2.15.0
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom
  2017-11-26 15:22 ` Leif Lindholm
@ 2017-11-27 13:02   ` Laszlo Ersek
  2017-11-27 16:57     ` Leif Lindholm
  0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2017-11-27 13:02 UTC (permalink / raw)
  To: Leif Lindholm, kalyan-nagabhirava
  Cc: edk2-devel, ard.biesheuvel, mark.gregotski

On 11/26/17 16:22, Leif Lindholm wrote:
> (Adding Laszlo to cc based on a single comment I make below.)
> 
> On Tue, Nov 21, 2017 at 04:23:36PM +0530, kalyan-nagabhirava wrote:

>>  [Guids.common]
>>    gHiKeyTokenSpaceGuid          =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
>> +  gHwTokenSpaceGuid             =  { 0x99999999, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
> 
> This very much looks like a not properly generated GUID.
> GUIDs must always be generated using an RFC4122-compliant algorithm.
> I generally recommend using
> https://www.guidgenerator.com/online-guid-generator.aspx.

I just run "uuidgen" in a terminal window.

>> +EFI_STATUS
>> +EFIAPI
>> +FvbSetAttributes(
>> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
>> +  IN OUT    EFI_FVB_ATTRIBUTES_2                 *Attributes
>> +  )
>> +{
>> +  DEBUG ((DEBUG_BLKIO, "FvbSetAttributes(0x%X) is not supported\n",*Attributes));
>> +  return EFI_UNSUPPORTED;
> 
> As per my (very) recent comment to Marcin, I do not believe returning
> EFI_UNSUPPORTED is a valid thing to do here. Which to me suggests the
> implementation of FvbGetAttributes is also incorrect.
> 
> Laszlo - what's your take on this in conjunction with PI 1.6 section
> 3.4.2? OvmfPkg does something very similar in
> EmuVariableFvbRuntimeDxe/Fvb.c.

I guess you are right. The particular OvmfPkg code that you mention is
likely also spec-breaking.

FWIW, in the OVMF flash driver that actually uses pflash, namely

  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c

the FvbSetVolumeAttributes() function appears both appropriate for the
spec and generic enough to copy elsewhere.

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom
  2017-11-27 13:02   ` Laszlo Ersek
@ 2017-11-27 16:57     ` Leif Lindholm
  2017-11-28  7:35       ` Kalyan Nagabhirava
  0 siblings, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2017-11-27 16:57 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: kalyan-nagabhirava, edk2-devel, ard.biesheuvel, mark.gregotski,
	Marcin Wojtas

On Mon, Nov 27, 2017 at 02:02:32PM +0100, Laszlo Ersek wrote:
> On 11/26/17 16:22, Leif Lindholm wrote:
> > (Adding Laszlo to cc based on a single comment I make below.)
> > 
> > On Tue, Nov 21, 2017 at 04:23:36PM +0530, kalyan-nagabhirava wrote:
> 
> >>  [Guids.common]
> >>    gHiKeyTokenSpaceGuid          =  { 0x91148425, 0xcdd2, 0x4830, { 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
> >> +  gHwTokenSpaceGuid             =  { 0x99999999, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
> > 
> > This very much looks like a not properly generated GUID.
> > GUIDs must always be generated using an RFC4122-compliant algorithm.
> > I generally recommend using
> > https://www.guidgenerator.com/online-guid-generator.aspx.
> 
> I just run "uuidgen" in a terminal window.

Yeah, I just prefer pointing to someone that does not require
installing anything, or requires specific operating systems.

> >> +EFI_STATUS
> >> +EFIAPI
> >> +FvbSetAttributes(
> >> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
> >> +  IN OUT    EFI_FVB_ATTRIBUTES_2                 *Attributes
> >> +  )
> >> +{
> >> +  DEBUG ((DEBUG_BLKIO, "FvbSetAttributes(0x%X) is not supported\n",*Attributes));
> >> +  return EFI_UNSUPPORTED;
> > 
> > As per my (very) recent comment to Marcin, I do not believe returning
> > EFI_UNSUPPORTED is a valid thing to do here. Which to me suggests the
> > implementation of FvbGetAttributes is also incorrect.
> > 
> > Laszlo - what's your take on this in conjunction with PI 1.6 section
> > 3.4.2? OvmfPkg does something very similar in
> > EmuVariableFvbRuntimeDxe/Fvb.c.
> 
> I guess you are right. The particular OvmfPkg code that you mention is
> likely also spec-breaking.
> 
> FWIW, in the OVMF flash driver that actually uses pflash, namely
> 
>   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
> 
> the FvbSetVolumeAttributes() function appears both appropriate for the
> spec and generic enough to copy elsewhere.

Yes, that looks good, thanks!

Marcin, Kalyan - please have a look at that implementation for
inspiration.

/
    Leif


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom
  2017-11-27 16:57     ` Leif Lindholm
@ 2017-11-28  7:35       ` Kalyan Nagabhirava
  2017-11-28 12:37         ` Leif Lindholm
  0 siblings, 1 reply; 7+ messages in thread
From: Kalyan Nagabhirava @ 2017-11-28  7:35 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: Laszlo Ersek, edk2-devel, Ard Biesheuvel, Mark Gregotski,
	Marcin Wojtas

HI Leif,
we didn't implemented enabling USB host and  Secure boot support  on Hikey
, we just took the code from openplatfrompkg (hikey branch)
 , we have implemented secureboot and DRI -disaster recovery image (HTTP
image download) application
and tested on HIkey platform , so for that purpose we are trying to
upstream the hikey code.

but hikey platform code  looks in bad shape (as per ard and your comments)
,so  we are planning to upstream
only our application code which is independent of platform.

Regards,
kalyan.


On 27 November 2017 at 22:27, Leif Lindholm <leif.lindholm@linaro.org>
wrote:

> On Mon, Nov 27, 2017 at 02:02:32PM +0100, Laszlo Ersek wrote:
> > On 11/26/17 16:22, Leif Lindholm wrote:
> > > (Adding Laszlo to cc based on a single comment I make below.)
> > >
> > > On Tue, Nov 21, 2017 at 04:23:36PM +0530, kalyan-nagabhirava wrote:
> >
> > >>  [Guids.common]
> > >>    gHiKeyTokenSpaceGuid          =  { 0x91148425, 0xcdd2, 0x4830, {
> 0x8b, 0xd0, 0xc6, 0x1c, 0x6d, 0xea, 0x36, 0x21 } }
> > >> +  gHwTokenSpaceGuid             =  { 0x99999999, 0x74c5, 0x4043, {
> 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
> > >
> > > This very much looks like a not properly generated GUID.
> > > GUIDs must always be generated using an RFC4122-compliant algorithm.
> > > I generally recommend using
> > > https://www.guidgenerator.com/online-guid-generator.aspx.
> >
> > I just run "uuidgen" in a terminal window.
>
> Yeah, I just prefer pointing to someone that does not require
> installing anything, or requires specific operating systems.
>
> > >> +EFI_STATUS
> > >> +EFIAPI
> > >> +FvbSetAttributes(
> > >> +  IN CONST  EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL  *This,
> > >> +  IN OUT    EFI_FVB_ATTRIBUTES_2                 *Attributes
> > >> +  )
> > >> +{
> > >> +  DEBUG ((DEBUG_BLKIO, "FvbSetAttributes(0x%X) is not
> supported\n",*Attributes));
> > >> +  return EFI_UNSUPPORTED;
> > >
> > > As per my (very) recent comment to Marcin, I do not believe returning
> > > EFI_UNSUPPORTED is a valid thing to do here. Which to me suggests the
> > > implementation of FvbGetAttributes is also incorrect.
> > >
> > > Laszlo - what's your take on this in conjunction with PI 1.6 section
> > > 3.4.2? OvmfPkg does something very similar in
> > > EmuVariableFvbRuntimeDxe/Fvb.c.
> >
> > I guess you are right. The particular OvmfPkg code that you mention is
> > likely also spec-breaking.
> >
> > FWIW, in the OVMF flash driver that actually uses pflash, namely
> >
> >   OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c
> >
> > the FvbSetVolumeAttributes() function appears both appropriate for the
> > spec and generic enough to copy elsewhere.
>
> Yes, that looks good, thanks!
>
> Marcin, Kalyan - please have a look at that implementation for
> inspiration.
>
> /
>     Leif
>



-- 
regards,
kalyan.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom
  2017-11-28  7:35       ` Kalyan Nagabhirava
@ 2017-11-28 12:37         ` Leif Lindholm
  2017-11-28 13:01           ` Kalyan Nagabhirava
  0 siblings, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2017-11-28 12:37 UTC (permalink / raw)
  To: Kalyan Nagabhirava
  Cc: Laszlo Ersek, edk2-devel, Ard Biesheuvel, Mark Gregotski

On Tue, Nov 28, 2017 at 01:05:35PM +0530, Kalyan Nagabhirava wrote:
> HI Leif,
> we didn't implemented enabling USB host and  Secure boot support  on Hikey
> , we just took the code from openplatfrompkg (hikey branch)

There is no hikey branch in OpenPlatformPkg.
If you are referring to some sort of fork somewhere, then please point
it out explicitly.

>  , we have implemented secureboot and DRI -disaster recovery image (HTTP
> image download) application
> and tested on HIkey platform , so for that purpose we are trying to
> upstream the hikey code.
> 
> but hikey platform code  looks in bad shape (as per ard and your comments)
> ,so  we are planning to upstream
> only our application code which is independent of platform.

I understand, but I cannot merge new code which is not used by any
upstream platform. If you do not have the time and bandwidth to clean
up these drivers, can you ask whoever looks after that fork to
upstream their platform support?

Best Regards,

Leif


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom
  2017-11-28 12:37         ` Leif Lindholm
@ 2017-11-28 13:01           ` Kalyan Nagabhirava
  0 siblings, 0 replies; 7+ messages in thread
From: Kalyan Nagabhirava @ 2017-11-28 13:01 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: Laszlo Ersek, edk2-devel, Ard Biesheuvel, Mark Gregotski

Hi Leif,

haojian zhuang from linaro  has shared below openplatformPkg repo  (branch
is hikey-wip) for Hikey platform to support USB host :

https://github.com/jlinton/OpenPlatformPkg

Regards,
kalyan.

On 28 November 2017 at 18:07, Leif Lindholm <leif.lindholm@linaro.org>
wrote:

> On Tue, Nov 28, 2017 at 01:05:35PM +0530, Kalyan Nagabhirava wrote:
> > HI Leif,
> > we didn't implemented enabling USB host and  Secure boot support  on
> Hikey
> > , we just took the code from openplatfrompkg (hikey branch)
>
> There is no hikey branch in OpenPlatformPkg.
> If you are referring to some sort of fork somewhere, then please point
> it out explicitly.
>
> >  , we have implemented secureboot and DRI -disaster recovery image (HTTP
> > image download) application
> > and tested on HIkey platform , so for that purpose we are trying to
> > upstream the hikey code.
> >
> > but hikey platform code  looks in bad shape (as per ard and your
> comments)
> > ,so  we are planning to upstream
> > only our application code which is independent of platform.
>
> I understand, but I cannot merge new code which is not used by any
> upstream platform. If you do not have the time and bandwidth to clean
> up these drivers, can you ask whoever looks after that fork to
> upstream their platform support?
>
> Best Regards,
>
> Leif
>



-- 
regards,
kalyan.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-11-28 12:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-21 10:53 [PATCH] [edk2-platforms]:Enabling Secure boot feature support on hikey platfrom kalyan-nagabhirava
2017-11-26 15:22 ` Leif Lindholm
2017-11-27 13:02   ` Laszlo Ersek
2017-11-27 16:57     ` Leif Lindholm
2017-11-28  7:35       ` Kalyan Nagabhirava
2017-11-28 12:37         ` Leif Lindholm
2017-11-28 13:01           ` Kalyan Nagabhirava

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox