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