* [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