public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/2] add boot options in HiKey
@ 2018-02-15 15:14 Haojian Zhuang
  2018-02-15 15:14 ` [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options Haojian Zhuang
  2018-02-15 15:14 ` [PATCH 2/2] Platform/Hisilicon/HiKey: " Haojian Zhuang
  0 siblings, 2 replies; 8+ messages in thread
From: Haojian Zhuang @ 2018-02-15 15:14 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm, ard.biesheuvel, linaro-uefi; +Cc: Haojian Zhuang

ChangeLog:
v1:
  *Create boot options in emu variable region.

Haojian Zhuang (2):
  Platform/Hisilicon/HiKey960: add boot options
  Platform/Hisilicon/HiKey: add boot options

 Platform/Hisilicon/HiKey/HiKey.dsc       |   3 +
 Platform/Hisilicon/HiKey/HiKey.fdf       | 239 +++++++++++++++++++++++++++++-
 Platform/Hisilicon/HiKey960/HiKey960.dsc |   3 +
 Platform/Hisilicon/HiKey960/HiKey960.fdf | 241 ++++++++++++++++++++++++++++++-
 4 files changed, 482 insertions(+), 4 deletions(-)

-- 
2.7.4



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

* [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options
  2018-02-15 15:14 [PATCH 0/2] add boot options in HiKey Haojian Zhuang
@ 2018-02-15 15:14 ` Haojian Zhuang
  2018-02-20 15:54   ` Laszlo Ersek
  2018-02-15 15:14 ` [PATCH 2/2] Platform/Hisilicon/HiKey: " Haojian Zhuang
  1 sibling, 1 reply; 8+ messages in thread
From: Haojian Zhuang @ 2018-02-15 15:14 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm, ard.biesheuvel, linaro-uefi; +Cc: Haojian Zhuang

Add four boot options in emu variable region. They are
"Boot on SD", "Grub", "Android Boot" and "Android Fastboot".

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 Platform/Hisilicon/HiKey960/HiKey960.dsc |   3 +
 Platform/Hisilicon/HiKey960/HiKey960.fdf | 241 ++++++++++++++++++++++++++++++-
 2 files changed, 242 insertions(+), 2 deletions(-)

diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc b/Platform/Hisilicon/HiKey960/HiKey960.dsc
index 98289c0..a6864c1 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
+++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
@@ -78,6 +78,9 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE
 
 [PcdsFixedAtBuild.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0x1AD88048
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x7FB8
+
   gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Alpha"
diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf b/Platform/Hisilicon/HiKey960/HiKey960.fdf
index 655032a..af10430 100644
--- a/Platform/Hisilicon/HiKey960/HiKey960.fdf
+++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf
@@ -26,12 +26,12 @@
 
 [FD.BL33_AP_UEFI]
 BaseAddress   = 0x1AC98000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
-Size          = 0x000F0000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
+Size          = 0x00100000|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     = 0x100
 
 ################################################################################
 #
@@ -53,6 +53,243 @@ NumBlocks     = 0xF0
 gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
 FV = FVMAIN_COMPACT
 
+0x000F0000|0x00008000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+DATA = {
+  ## This is the EFI_FIRMWARE_VOLUME_HEADER
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+  # FileSystemGuid: gEfiSystemNvDataFvGuid         =
+  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
+  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
+  # FvLength: 0x8000
+  0x00, 0x80, 0x0, 0x00, 0x00, 0x00, 0x00, 0x00,
+  #Signature "_FVH"       #Attributes
+  0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
+  #HeaderLength #CheckSum #ExtHeaderOffset #Reserved #Revision
+  0x48, 0x00, 0x36, 0x09, 0x00, 0x00, 0x00, 0x02,
+  #Blockmap[0]: 8 Blocks * 0x1000 Bytes / Block
+  0x08, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
+  #Blockmap[1]: End
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+  ## end of EFI_FIRMWARE_VOLUME_HEADER.
+
+  ### Offset (0x48) ###
+  ## This is the VARIABLE_STORE_HEADER gEfiVariableGuid
+  0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
+  0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
+  #Size: 0x8000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x7FB8
+  0xB8, 0x7F, 0x00, 0x00,
+  #FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
+  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+
+  ### Offset (0x64) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x14, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x18, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  ## End of VARIABLE_HEADER. Offset (0x84)
+  #VariableName: BootOrder
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x4F, 0x00, 0x72, 0x00, 0x64, 0x00, 0x65, 0x00,
+  0x72, 0x00, 0x00, 0x00,
+  #Data
+  0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x03, 0x00,
+  0x04, 0x00, 0x05, 0x00, 0x06, 0x00, 0x07, 0x00,
+  0x08, 0x00, 0x09, 0x00, 0x0a, 0x00, 0x0b, 0x00,
+  ### End of BootOrder.
+
+  ### Offset (0xB0) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x12, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x42, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  #VariableName: Boot0000
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
+  0x00, 0x00,
+  ###  Offset (0xE2), Size(0x32) ###
+  #Data:
+  #Attributes: LOAD_OPTION_ACTIVE
+  0x01, 0x00, 0x00, 0x00,
+  #FilePathListLength
+  0x26, 0x00,
+  #Description: "Boot on SD"
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #Boot
+  0x20, 0x00, 0x6F, 0x00, 0x6E, 0x00, 0x20, 0x00,      # on 
+  0x53, 0x00, 0x44, 0x00, 0x00, 0x00,                  #SD
+  #FilePathList (0x26)
+  0x01, 0x04, 0x1D, 0x00, 0x5B, 0x90, 0x51, 0x0D,
+  0x7E, 0xB7, 0x2A, 0x45, 0xA2, 0xC0, 0xEC, 0xA0,
+  0xCC, 0x8D, 0x51, 0x4A, 0x00, 0xF0, 0x37, 0xFF,
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x1A, 0x05,
+  0x00, 0x00, 0x7F, 0xFF, 0x04, 0x00,
+  ### Offset (0x124), Size (0x42) ###
+  ### End of Boot0000
+
+  ### Offset (0x124) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x12, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x93, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  #VariableName: Boot0001
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x31, 0x00,
+  0x00, 0x00,
+  ### Offset (0x156). Size (0x32) ###
+  #Data:
+  #Attributes: LOAD_OPTION_ACTIVE
+  0x01, 0x00, 0x00, 0x00,
+  #FilePathListLength
+  0x83, 0x00,
+  #Description: "Grub"
+  0x47, 0x00, 0x72, 0x00, 0x75, 0x00, 0x62, 0x00,      #Grub
+  0x00, 0x00,
+  #FilePathList (0x83)
+  0x01, 0x04, 0x1D, 0x00, 0x5B, 0x90, 0x51, 0x0D,
+  0x7E, 0xB7, 0x2A, 0x45, 0xA2, 0xC0, 0xEC, 0xA0,
+  0xCC, 0x8D, 0x51, 0x4A, 0x00, 0x00, 0x3B, 0xFF,
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x19, 0x06,
+  0x00, 0x00, 0x03, 0x04, 0x01, 0x2A, 0x00, 0x07,
+  0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x96, 0x06, 0x34, 0xD3, 0x95,
+  0x9B, 0x64, 0x4C, 0x8D, 0xF6, 0xE6, 0xD4, 0x54,
+  0x8F, 0xBA, 0x41, 0x02, 0x02, 0x04, 0x04, 0x32,
+  0x00, 0x5C, 0x00, 0x45, 0x00, 0x46, 0x00, 0x49,
+  0x00, 0x5C, 0x00, 0x42, 0x00, 0x4F, 0x00, 0x4F,
+  0x00, 0x54, 0x00, 0x5C, 0x00, 0x47, 0x00, 0x52,
+  0x00, 0x55, 0x00, 0x42, 0x00, 0x41, 0x00, 0x41,
+  0x00, 0x36, 0x00, 0x34, 0x00, 0x2E, 0x00, 0x45,
+  0x00, 0x46, 0x00, 0x49, 0x00, 0x00, 0x00, 0x7F,
+  0xFF, 0x04, 0x00,
+  ### Offset (0x1E9). Size (0x93) ###
+  ## End of Boot0001
+  ### PADDING ###
+  0x00, 0x00, 0x00,
+
+  ### Offset (0x1EC) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x12, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x50, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  #VariableName: Boot0002
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x32, 0x00,
+  0x00, 0x00,
+  ### Offset (0x220). Size (0x32) ###
+  #Data:
+  #Attributes: LOAD_OPTION_ACTIVE
+  0x01, 0x00, 0x00, 0x00,
+  #FilePathListLength
+  0x30, 0x00,
+  #Description: "Android Boot"
+  0x41, 0x00, 0x6E, 0x00, 0x64, 0x00, 0x72, 0x00,      #Andr
+  0x6F, 0x00, 0x69, 0x00, 0x64, 0x00, 0x20, 0x00,      #oid 
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #Boot
+  0x00, 0x00,
+  #FilePathList (AndroidBoot)
+  0x01, 0x03, 0x18, 0x00, 0x0B, 0x00, 0x00, 0x00,
+  0x00, 0x90, 0x20, 0xBF, 0x00, 0x00, 0x00, 0x00,
+  0xBF, 0xDE, 0x8E, 0xBF, 0x00, 0x00, 0x00, 0x00,
+  0x04, 0x06, 0x14, 0x00, 0x36, 0x8B, 0x73, 0x3A,
+  0xC5, 0xB9, 0x63, 0x47, 0xAB, 0xBD, 0x6C, 0xBD,
+  0x4B, 0x25, 0xF9, 0xFF, 0x7F, 0xFF, 0x04, 0x00,
+  ### Offset (0x26E). Size (0x50) ###
+  ## End of Boot0002
+  ### PADDING ###
+  0x00, 0x00,
+
+  ### Offset (0x270) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x12, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x58, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  #VariableName: Boot0003
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x33, 0x00,
+  0x00, 0x00,
+  ### Offset (0x2A2). Size (0x32) ###
+  #Data:
+  #Attributes: LOAD_OPTION_ACTIVE
+  0x01, 0x00, 0x00, 0x00,
+  #FilePathListLength
+  0x30, 0x00,
+  #Description: "Android Fastboot"
+  0x41, 0x00, 0x6E, 0x00, 0x64, 0x00, 0x72, 0x00,      #Andr
+  0x6F, 0x00, 0x69, 0x00, 0x64, 0x00, 0x20, 0x00,      #oid 
+  0x46, 0x00, 0x61, 0x00, 0x73, 0x00, 0x74, 0x00,      #Fast
+  0x62, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #boot
+  0x00, 0x00,
+  #FilePathList (AndroidFastboot)
+  0x01, 0x03, 0x18, 0x00, 0x0B, 0x00, 0x00, 0x00,
+  0x00, 0x90, 0x20, 0xBF, 0x00, 0x00, 0x00, 0x00,
+  0xBF, 0xDE, 0x8E, 0xBF, 0x00, 0x00, 0x00, 0x00,
+  0x04, 0x06, 0x14, 0x00, 0x2A, 0x50, 0x88, 0x95,
+  0x70, 0x53, 0xE3, 0x11, 0x86, 0x31, 0xD7, 0xC5,
+  0x95, 0x13, 0x64, 0xC8, 0x7F, 0xFF, 0x04, 0x00,
+  ## Offset (0x2FA). Size (0x58) ###
+  ## End of Boot0003
+
+  0x00
+}
+
+0x000F8000|0x00002000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+#NV_FTW_WORKING
+DATA = {
+  # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid          =
+  0x2B, 0x29, 0x58, 0x9E, 0x68, 0x7C, 0x7D, 0x49,
+  0xA0, 0xCE, 0x65, 0x0 , 0xFD, 0x9F, 0x1B, 0x95,
+  # Crc:UINT32            #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
+  0xE2, 0x33, 0xF2, 0x03, 0xFE, 0xFF, 0xFF, 0xFF,
+  # WriteQueueSize: UINT64 #Size: 0x2000 - 0x20 (FTW_WORKING_HEADER) = 0x1FE0
+  0xE0, 0x1F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+}
+
+0x000FA000|0x00002000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+
 
 ################################################################################
 #
-- 
2.7.4



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

* [PATCH 2/2] Platform/Hisilicon/HiKey: add boot options
  2018-02-15 15:14 [PATCH 0/2] add boot options in HiKey Haojian Zhuang
  2018-02-15 15:14 ` [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options Haojian Zhuang
@ 2018-02-15 15:14 ` Haojian Zhuang
  1 sibling, 0 replies; 8+ messages in thread
From: Haojian Zhuang @ 2018-02-15 15:14 UTC (permalink / raw)
  To: edk2-devel, leif.lindholm, ard.biesheuvel, linaro-uefi; +Cc: Haojian Zhuang

Add four boot options into emu variable region. They're
"Boot on SD", "Grub", "Android Boot" and "Android Fastboot".

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 Platform/Hisilicon/HiKey/HiKey.dsc |   3 +
 Platform/Hisilicon/HiKey/HiKey.fdf | 239 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 240 insertions(+), 2 deletions(-)

diff --git a/Platform/Hisilicon/HiKey/HiKey.dsc b/Platform/Hisilicon/HiKey/HiKey.dsc
index b0f8a93..7db06b2 100644
--- a/Platform/Hisilicon/HiKey/HiKey.dsc
+++ b/Platform/Hisilicon/HiKey/HiKey.dsc
@@ -81,6 +81,9 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE
 
 [PcdsFixedAtBuild.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0x350F0048
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x7FB8
+
   gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Alpha"
diff --git a/Platform/Hisilicon/HiKey/HiKey.fdf b/Platform/Hisilicon/HiKey/HiKey.fdf
index 2a5c5a4..80b272c 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          = 0x00100000|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     = 0x100
 
 ################################################################################
 #
@@ -53,6 +53,241 @@ NumBlocks     = 0xF0
 gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
 FV = FVMAIN_COMPACT
 
+0x000F0000|0x00008000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+DATA = {
+  ## This is the EFI_FIRMWARE_VOLUME_HEADER
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+  # FileSystemGuid: gEfiSystemNvDataFvGuid         =
+  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
+  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
+  # FvLength: 0x8000
+  0x00, 0x80, 0x0, 0x00, 0x00, 0x00, 0x00, 0x00,
+  #Signature "_FVH"       #Attributes
+  0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
+  #HeaderLength #CheckSum #ExtHeaderOffset #Reserved #Revision
+  0x48, 0x00, 0x36, 0x09, 0x00, 0x00, 0x00, 0x02,
+  #Blockmap[0]: 8 Blocks * 0x1000 Bytes / Block
+  0x08, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
+  #Blockmap[1]: End
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+  ## end of EFI_FIRMWARE_VOLUME_HEADER.
+
+  ### Offset (0x48) ###
+  ## This is the VARIABLE_STORE_HEADER gEfiVariableGuid
+  0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
+  0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
+  #Size: 0x8000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x7FB8
+  0xB8, 0x7F, 0x00, 0x00,
+  #FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
+  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+
+  ### Offset (0x64) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x14, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x18, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  ## End of VARIABLE_HEADER. Offset (0x84)
+  #VariableName: BootOrder
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x4F, 0x00, 0x72, 0x00, 0x64, 0x00, 0x65, 0x00,
+  0x72, 0x00, 0x00, 0x00,
+  #Data
+  0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x03, 0x00,
+  0x04, 0x00, 0x05, 0x00, 0x06, 0x00, 0x07, 0x00,
+  0x08, 0x00, 0x09, 0x00, 0x0a, 0x00, 0x0b, 0x00,
+  ### End of BootOrder.
+
+  ### Offset (0xB0) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x12, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x42, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  #VariableName: Boot0000
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
+  0x00, 0x00,
+  ###  Offset (0xE2), Size(0x32) ###
+  #Data:
+  #Attributes: LOAD_OPTION_ACTIVE
+  0x01, 0x00, 0x00, 0x00,
+  #FilePathListLength
+  0x26, 0x00,
+  #Description: "Boot on SD"
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #Boot
+  0x20, 0x00, 0x6F, 0x00, 0x6E, 0x00, 0x20, 0x00,      # on 
+  0x53, 0x00, 0x44, 0x00, 0x00, 0x00,                  #SD
+  #FilePathList (0x26)
+  0x01, 0x04, 0x1D, 0x00, 0x5B, 0x90, 0x51, 0x0D,
+  0x7E, 0xB7, 0x2A, 0x45, 0xA2, 0xC0, 0xEC, 0xA0,
+  0xCC, 0x8D, 0x51, 0x4A, 0x00, 0xE0, 0x23, 0xF7,
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x1A, 0x05,
+  0x00, 0x00, 0x7F, 0xFF, 0x04, 0x00,
+  ### Offset (0x124), Size (0x42) ###
+  ### End of Boot0000
+
+  ### Offset (0x124) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x12, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x9A, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  #VariableName: Boot0001
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x31, 0x00,
+  0x00, 0x00,
+  ### Offset (0x156). Size (0x32) ###
+  #Data:
+  #Attributes: LOAD_OPTION_ACTIVE
+  0x01, 0x00, 0x00, 0x00,
+  #FilePathListLength
+  0x8A, 0x00,
+  #Description: "Grub"
+  0x47, 0x00, 0x72, 0x00, 0x75, 0x00, 0x62, 0x00,      #Grub
+  0x00, 0x00,
+  #FilePathList (0x8A)
+  0x01, 0x04, 0x1D, 0x00, 0x5B, 0x90, 0x51, 0x0D,
+  0x7E, 0xB7, 0x2A, 0x45, 0xA2, 0xC0, 0xEC, 0xA0,
+  0xCC, 0x8D, 0x51, 0x4A, 0x00, 0xD0, 0x23, 0xF7,
+  0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x1D, 0x05,
+  0x00, 0x00, 0x01, 0x05, 0x08, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x04, 0x01, 0x2A, 0x00, 0x06, 0x00,
+  0x00, 0x00, 0x00, 0x70, 0x00, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
+  0x00, 0x00, 0x3C, 0x21, 0x0F, 0x5C, 0xE1, 0x17,
+  0x49, 0x41, 0x88, 0xC8, 0x8B, 0x50, 0xFB, 0x4E,
+  0xC7, 0x0E, 0x02, 0x02, 0x04, 0x04, 0x32, 0x00,
+  0x5C, 0x00, 0x45, 0x00, 0x46, 0x00, 0x49, 0x00,
+  0x5C, 0x00, 0x42, 0x00, 0x4F, 0x00, 0x4F, 0x00,
+  0x54, 0x00, 0x5C, 0x00, 0x47, 0x00, 0x52, 0x00,
+  0x55, 0x00, 0x42, 0x00, 0x41, 0x00, 0x41, 0x00,
+  0x36, 0x00, 0x34, 0x00, 0x2E, 0x00, 0x45, 0x00,
+  0x46, 0x00, 0x49, 0x00, 0x00, 0x00, 0x7F, 0xFF,
+  0x04, 0x00,
+  ### Offset (0x1F0). Size (0x9A) ###
+  ## End of Boot0001
+
+  ### Offset (0x1F0) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x12, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x4C, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  #VariableName: Boot0002
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x32, 0x00,
+  0x00, 0x00,
+  ### Offset (0x222). Size (0x32) ###
+  #Data:
+  #Attributes: LOAD_OPTION_ACTIVE
+  0x01, 0x00, 0x00, 0x00,
+  #FilePathListLength
+  0x2C, 0x00,
+  #Description: "Android Boot"
+  0x41, 0x00, 0x6E, 0x00, 0x64, 0x00, 0x72, 0x00,      #Andr
+  0x6F, 0x00, 0x69, 0x00, 0x64, 0x00, 0x20, 0x00,      #oid 
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #Boot
+  0x00, 0x00,
+  #FilePathList (AndroidBoot)
+  0x04, 0x07, 0x14, 0x00, 0x69, 0xD4, 0xB7, 0x69,
+  0xA2, 0x55, 0xD8, 0x49, 0xA4, 0x26, 0x42, 0xBF,
+  0xB2, 0x2F, 0x5B, 0x9D, 0x04, 0x06, 0x14, 0x00,
+  0x36, 0x8B, 0x73, 0x3A, 0xC5, 0xB9, 0x63, 0x47,
+  0xAB, 0xBD, 0x6C, 0xBD, 0x4B, 0x25, 0xF9, 0xFF,
+  0x7F, 0xFF, 0x04, 0x00,
+  ### Offset (0x26E). Size (0x4C) ###
+  ## End of Boot0002
+  ### PADDING ###
+  0x00, 0x00,
+
+  ### Offset (0x270) ###
+  ## This is the VARIABLE_HEADER
+  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
+  0xAA, 0x55, 0x3F, 0x00,
+  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
+  0x03, 0x00, 0x00, 0x00,
+  #NameSize:
+  0x12, 0x00, 0x00, 0x00,
+  #DataSize:
+  0x54, 0x00, 0x00, 0x00,
+  #VariableGuid: gEfiGlobalVariableGuid
+  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
+  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
+  #VariableName: Boot0003
+  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
+  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x33, 0x00,
+  0x00, 0x00,
+  ### Offset (0x2A2). Size (0x32) ###
+  #Data:
+  #Attributes: LOAD_OPTION_ACTIVE
+  0x01, 0x00, 0x00, 0x00,
+  #FilePathListLength
+  0x2C, 0x00,
+  #Description: "Android Fastboot"
+  0x41, 0x00, 0x6E, 0x00, 0x64, 0x00, 0x72, 0x00,      #Andr
+  0x6F, 0x00, 0x69, 0x00, 0x64, 0x00, 0x20, 0x00,      #oid 
+  0x46, 0x00, 0x61, 0x00, 0x73, 0x00, 0x74, 0x00,      #Fast
+  0x62, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #boot
+  0x00, 0x00,
+  #FilePathList (AndroidFastboot)
+  0x04, 0x07, 0x14, 0x00, 0x69, 0xD4, 0xB7, 0x69,
+  0xA2, 0x55, 0xD8, 0x49, 0xA4, 0x26, 0x42, 0xBF,
+  0xB2, 0x2F, 0x5B, 0x9D, 0x04, 0x06, 0x14, 0x00,
+  0x2A, 0x50, 0x88, 0x95, 0x70, 0x53, 0xE3, 0x11,
+  0x86, 0x31, 0xD7, 0xC5, 0x95, 0x13, 0x64, 0xC8,
+  0x7F, 0xFF, 0x04, 0x00,
+  ## Offset (0x2F6). Size (0x54) ###
+  ## End of Boot0003
+
+  0x00
+}
+
+0x000F8000|0x00002000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+#NV_FTW_WORKING
+DATA = {
+  # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid          =
+  0x2B, 0x29, 0x58, 0x9E, 0x68, 0x7C, 0x7D, 0x49,
+  0xA0, 0xCE, 0x65, 0x0 , 0xFD, 0x9F, 0x1B, 0x95,
+  # Crc:UINT32            #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
+  0xE2, 0x33, 0xF2, 0x03, 0xFE, 0xFF, 0xFF, 0xFF,
+  # WriteQueueSize: UINT64 #Size: 0x2000 - 0x20 (FTW_WORKING_HEADER) = 0x1FE0
+  0xE0, 0x1F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+}
+
+0x000FA000|0x00002000
+gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
 
 ################################################################################
 #
-- 
2.7.4



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

* Re: [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options
  2018-02-15 15:14 ` [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options Haojian Zhuang
@ 2018-02-20 15:54   ` Laszlo Ersek
  2018-02-22  8:57     ` Haojian Zhuang
  0 siblings, 1 reply; 8+ messages in thread
From: Laszlo Ersek @ 2018-02-20 15:54 UTC (permalink / raw)
  To: Haojian Zhuang; +Cc: edk2-devel, leif.lindholm, ard.biesheuvel, linaro-uefi

Hi,

On 02/15/18 16:14, Haojian Zhuang wrote:
> Add four boot options in emu variable region. They are
> "Boot on SD", "Grub", "Android Boot" and "Android Fastboot".
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
> ---
>  Platform/Hisilicon/HiKey960/HiKey960.dsc |   3 +
>  Platform/Hisilicon/HiKey960/HiKey960.fdf | 241 ++++++++++++++++++++++++++++++-
>  2 files changed, 242 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc b/Platform/Hisilicon/HiKey960/HiKey960.dsc
> index 98289c0..a6864c1 100644
> --- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
> +++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
> @@ -78,6 +78,9 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE
>  
>  [PcdsFixedAtBuild.common]
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0x1AD88048
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x7FB8
> +
>    gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Alpha"
> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf b/Platform/Hisilicon/HiKey960/HiKey960.fdf
> index 655032a..af10430 100644
> --- a/Platform/Hisilicon/HiKey960/HiKey960.fdf
> +++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf
> @@ -26,12 +26,12 @@
>  
>  [FD.BL33_AP_UEFI]
>  BaseAddress   = 0x1AC98000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
> -Size          = 0x000F0000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
> +Size          = 0x00100000|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     = 0x100
>  
>  ################################################################################
>  #
> @@ -53,6 +53,243 @@ NumBlocks     = 0xF0
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>  
> +0x000F0000|0x00008000
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> +DATA = {
> +  ## This is the EFI_FIRMWARE_VOLUME_HEADER
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  # FileSystemGuid: gEfiSystemNvDataFvGuid         =
> +  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
> +  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
> +  # FvLength: 0x8000
> +  0x00, 0x80, 0x0, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  #Signature "_FVH"       #Attributes
> +  0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
> +  #HeaderLength #CheckSum #ExtHeaderOffset #Reserved #Revision
> +  0x48, 0x00, 0x36, 0x09, 0x00, 0x00, 0x00, 0x02,
> +  #Blockmap[0]: 8 Blocks * 0x1000 Bytes / Block
> +  0x08, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
> +  #Blockmap[1]: End
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +  ## end of EFI_FIRMWARE_VOLUME_HEADER.
> +
> +  ### Offset (0x48) ###
> +  ## This is the VARIABLE_STORE_HEADER gEfiVariableGuid
> +  0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
> +  0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
> +  #Size: 0x8000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x7FB8
> +  0xB8, 0x7F, 0x00, 0x00,
> +  #FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
> +  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +
> +  ### Offset (0x64) ###
> +  ## This is the VARIABLE_HEADER
> +  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
> +  0xAA, 0x55, 0x3F, 0x00,
> +  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
> +  0x03, 0x00, 0x00, 0x00,
> +  #NameSize:
> +  0x14, 0x00, 0x00, 0x00,
> +  #DataSize:
> +  0x18, 0x00, 0x00, 0x00,
> +  #VariableGuid: gEfiGlobalVariableGuid
> +  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
> +  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
> +  ## End of VARIABLE_HEADER. Offset (0x84)
> +  #VariableName: BootOrder
> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
> +  0x4F, 0x00, 0x72, 0x00, 0x64, 0x00, 0x65, 0x00,
> +  0x72, 0x00, 0x00, 0x00,
> +  #Data
> +  0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x03, 0x00,
> +  0x04, 0x00, 0x05, 0x00, 0x06, 0x00, 0x07, 0x00,
> +  0x08, 0x00, 0x09, 0x00, 0x0a, 0x00, 0x0b, 0x00,
> +  ### End of BootOrder.
> +
> +  ### Offset (0xB0) ###
> +  ## This is the VARIABLE_HEADER
> +  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
> +  0xAA, 0x55, 0x3F, 0x00,
> +  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
> +  0x03, 0x00, 0x00, 0x00,
> +  #NameSize:
> +  0x12, 0x00, 0x00, 0x00,
> +  #DataSize:
> +  0x42, 0x00, 0x00, 0x00,
> +  #VariableGuid: gEfiGlobalVariableGuid
> +  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
> +  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
> +  #VariableName: Boot0000
> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
> +  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
> +  0x00, 0x00,
> +  ###  Offset (0xE2), Size(0x32) ###
> +  #Data:
> +  #Attributes: LOAD_OPTION_ACTIVE
> +  0x01, 0x00, 0x00, 0x00,
> +  #FilePathListLength
> +  0x26, 0x00,
> +  #Description: "Boot on SD"
> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #Boot
> +  0x20, 0x00, 0x6F, 0x00, 0x6E, 0x00, 0x20, 0x00,      # on 
> +  0x53, 0x00, 0x44, 0x00, 0x00, 0x00,                  #SD
> +  #FilePathList (0x26)
> +  0x01, 0x04, 0x1D, 0x00, 0x5B, 0x90, 0x51, 0x0D,
> +  0x7E, 0xB7, 0x2A, 0x45, 0xA2, 0xC0, 0xEC, 0xA0,
> +  0xCC, 0x8D, 0x51, 0x4A, 0x00, 0xF0, 0x37, 0xFF,
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x1A, 0x05,
> +  0x00, 0x00, 0x7F, 0xFF, 0x04, 0x00,
> +  ### Offset (0x124), Size (0x42) ###
> +  ### End of Boot0000
> +
> +  ### Offset (0x124) ###
> +  ## This is the VARIABLE_HEADER
> +  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
> +  0xAA, 0x55, 0x3F, 0x00,
> +  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
> +  0x03, 0x00, 0x00, 0x00,
> +  #NameSize:
> +  0x12, 0x00, 0x00, 0x00,
> +  #DataSize:
> +  0x93, 0x00, 0x00, 0x00,
> +  #VariableGuid: gEfiGlobalVariableGuid
> +  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
> +  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
> +  #VariableName: Boot0001
> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
> +  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x31, 0x00,
> +  0x00, 0x00,
> +  ### Offset (0x156). Size (0x32) ###
> +  #Data:
> +  #Attributes: LOAD_OPTION_ACTIVE
> +  0x01, 0x00, 0x00, 0x00,
> +  #FilePathListLength
> +  0x83, 0x00,
> +  #Description: "Grub"
> +  0x47, 0x00, 0x72, 0x00, 0x75, 0x00, 0x62, 0x00,      #Grub
> +  0x00, 0x00,
> +  #FilePathList (0x83)
> +  0x01, 0x04, 0x1D, 0x00, 0x5B, 0x90, 0x51, 0x0D,
> +  0x7E, 0xB7, 0x2A, 0x45, 0xA2, 0xC0, 0xEC, 0xA0,
> +  0xCC, 0x8D, 0x51, 0x4A, 0x00, 0x00, 0x3B, 0xFF,
> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x19, 0x06,
> +  0x00, 0x00, 0x03, 0x04, 0x01, 0x2A, 0x00, 0x07,
> +  0x00, 0x00, 0x00, 0x00, 0x21, 0x01, 0x00, 0x00,
> +  0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00,
> +  0x00, 0x00, 0x00, 0x96, 0x06, 0x34, 0xD3, 0x95,
> +  0x9B, 0x64, 0x4C, 0x8D, 0xF6, 0xE6, 0xD4, 0x54,
> +  0x8F, 0xBA, 0x41, 0x02, 0x02, 0x04, 0x04, 0x32,
> +  0x00, 0x5C, 0x00, 0x45, 0x00, 0x46, 0x00, 0x49,
> +  0x00, 0x5C, 0x00, 0x42, 0x00, 0x4F, 0x00, 0x4F,
> +  0x00, 0x54, 0x00, 0x5C, 0x00, 0x47, 0x00, 0x52,
> +  0x00, 0x55, 0x00, 0x42, 0x00, 0x41, 0x00, 0x41,
> +  0x00, 0x36, 0x00, 0x34, 0x00, 0x2E, 0x00, 0x45,
> +  0x00, 0x46, 0x00, 0x49, 0x00, 0x00, 0x00, 0x7F,
> +  0xFF, 0x04, 0x00,
> +  ### Offset (0x1E9). Size (0x93) ###
> +  ## End of Boot0001
> +  ### PADDING ###
> +  0x00, 0x00, 0x00,
> +
> +  ### Offset (0x1EC) ###
> +  ## This is the VARIABLE_HEADER
> +  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
> +  0xAA, 0x55, 0x3F, 0x00,
> +  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
> +  0x03, 0x00, 0x00, 0x00,
> +  #NameSize:
> +  0x12, 0x00, 0x00, 0x00,
> +  #DataSize:
> +  0x50, 0x00, 0x00, 0x00,
> +  #VariableGuid: gEfiGlobalVariableGuid
> +  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
> +  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
> +  #VariableName: Boot0002
> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
> +  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x32, 0x00,
> +  0x00, 0x00,
> +  ### Offset (0x220). Size (0x32) ###
> +  #Data:
> +  #Attributes: LOAD_OPTION_ACTIVE
> +  0x01, 0x00, 0x00, 0x00,
> +  #FilePathListLength
> +  0x30, 0x00,
> +  #Description: "Android Boot"
> +  0x41, 0x00, 0x6E, 0x00, 0x64, 0x00, 0x72, 0x00,      #Andr
> +  0x6F, 0x00, 0x69, 0x00, 0x64, 0x00, 0x20, 0x00,      #oid 
> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #Boot
> +  0x00, 0x00,
> +  #FilePathList (AndroidBoot)
> +  0x01, 0x03, 0x18, 0x00, 0x0B, 0x00, 0x00, 0x00,
> +  0x00, 0x90, 0x20, 0xBF, 0x00, 0x00, 0x00, 0x00,
> +  0xBF, 0xDE, 0x8E, 0xBF, 0x00, 0x00, 0x00, 0x00,
> +  0x04, 0x06, 0x14, 0x00, 0x36, 0x8B, 0x73, 0x3A,
> +  0xC5, 0xB9, 0x63, 0x47, 0xAB, 0xBD, 0x6C, 0xBD,
> +  0x4B, 0x25, 0xF9, 0xFF, 0x7F, 0xFF, 0x04, 0x00,
> +  ### Offset (0x26E). Size (0x50) ###
> +  ## End of Boot0002
> +  ### PADDING ###
> +  0x00, 0x00,
> +
> +  ### Offset (0x270) ###
> +  ## This is the VARIABLE_HEADER
> +  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
> +  0xAA, 0x55, 0x3F, 0x00,
> +  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
> +  0x03, 0x00, 0x00, 0x00,
> +  #NameSize:
> +  0x12, 0x00, 0x00, 0x00,
> +  #DataSize:
> +  0x58, 0x00, 0x00, 0x00,
> +  #VariableGuid: gEfiGlobalVariableGuid
> +  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
> +  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
> +  #VariableName: Boot0003
> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
> +  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x33, 0x00,
> +  0x00, 0x00,
> +  ### Offset (0x2A2). Size (0x32) ###
> +  #Data:
> +  #Attributes: LOAD_OPTION_ACTIVE
> +  0x01, 0x00, 0x00, 0x00,
> +  #FilePathListLength
> +  0x30, 0x00,
> +  #Description: "Android Fastboot"
> +  0x41, 0x00, 0x6E, 0x00, 0x64, 0x00, 0x72, 0x00,      #Andr
> +  0x6F, 0x00, 0x69, 0x00, 0x64, 0x00, 0x20, 0x00,      #oid 
> +  0x46, 0x00, 0x61, 0x00, 0x73, 0x00, 0x74, 0x00,      #Fast
> +  0x62, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #boot
> +  0x00, 0x00,
> +  #FilePathList (AndroidFastboot)
> +  0x01, 0x03, 0x18, 0x00, 0x0B, 0x00, 0x00, 0x00,
> +  0x00, 0x90, 0x20, 0xBF, 0x00, 0x00, 0x00, 0x00,
> +  0xBF, 0xDE, 0x8E, 0xBF, 0x00, 0x00, 0x00, 0x00,
> +  0x04, 0x06, 0x14, 0x00, 0x2A, 0x50, 0x88, 0x95,
> +  0x70, 0x53, 0xE3, 0x11, 0x86, 0x31, 0xD7, 0xC5,
> +  0x95, 0x13, 0x64, 0xC8, 0x7F, 0xFF, 0x04, 0x00,
> +  ## Offset (0x2FA). Size (0x58) ###
> +  ## End of Boot0003
> +
> +  0x00
> +}
> +
> +0x000F8000|0x00002000
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> +#NV_FTW_WORKING
> +DATA = {
> +  # EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER->Signature = gEdkiiWorkingBlockSignatureGuid          =
> +  0x2B, 0x29, 0x58, 0x9E, 0x68, 0x7C, 0x7D, 0x49,
> +  0xA0, 0xCE, 0x65, 0x0 , 0xFD, 0x9F, 0x1B, 0x95,
> +  # Crc:UINT32            #WorkingBlockValid:1, WorkingBlockInvalid:1, Reserved
> +  0xE2, 0x33, 0xF2, 0x03, 0xFE, 0xFF, 0xFF, 0xFF,
> +  # WriteQueueSize: UINT64 #Size: 0x2000 - 0x20 (FTW_WORKING_HEADER) = 0x1FE0
> +  0xE0, 0x1F, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +}
> +
> +0x000FA000|0x00002000
> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
> +
>  
>  ################################################################################
>  #
> 

While in both ArmVirtPkg and OvmfPkg we use a similar DATA definition
for *formatting* the pristine variable store (just setting the
absolutely necessary headers via a hexdump), I think that adding
"business content" like this is a bad idea. For one, if the defaults
have to be updated ever again, the patches for the DATA definitions will
look terrible. It's hard to review and maintain hexdump like these; we
should keep such DATA definitions to the absolute minimum.

Such "default" boot options should be set by the platform's
PlatformBootManagerLib instance. PlatformBootManagerLib is responsible
for generating boot options. In doing that, PlatformBootManagerLib can
call helper functions from UefiBootManagerLib, so everything need not be
done manually.

For example, EfiBootManagerGetLoadOptions() can be used to retrieve the
current boot options. Then, you can iterate over the four boot options
that you want to ensure exist -- for each:

- create a EFI_BOOT_MANAGER_LOAD_OPTION object with
EfiBootManagerInitializeLoadOption(),
- check if the option already exists, with EfiBootManagerFindLoadOption(),
- if the option doesn't exist yet, add it with
EfiBootManagerAddLoadOptionVariable(),
- free the EFI_BOOT_MANAGER_LOAD_OPTION object with
EfiBootManagerFreeLoadOption()

Finally, free the originally retrieved set of boot options with
EfiBootManagerFreeLoadOptions().

You can find example code in
"ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c".

Thanks,
Laszlo


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

* Re: [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options
  2018-02-20 15:54   ` Laszlo Ersek
@ 2018-02-22  8:57     ` Haojian Zhuang
  2018-02-22 10:56       ` Laszlo Ersek
  0 siblings, 1 reply; 8+ messages in thread
From: Haojian Zhuang @ 2018-02-22  8:57 UTC (permalink / raw)
  To: lersek@redhat.com, Haojian Zhuang
  Cc: edk2-devel@lists.01.org, leif.lindholm@linaro.org,
	ard.biesheuvel@linaro.org, linaro-uefi@lists.linaro.org

On 02/20/2018 11:54 PM, Laszlo Ersek wrote:
> Hi,
> 
> On 02/15/18 16:14, Haojian Zhuang wrote:
>> Add four boot options in emu variable region. They are
>> "Boot on SD", "Grub", "Android Boot" and "Android Fastboot".
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> ---
>>   Platform/Hisilicon/HiKey960/HiKey960.dsc |   3 +
>>   Platform/Hisilicon/HiKey960/HiKey960.fdf | 241 ++++++++++++++++++++++++++++++-
>>   2 files changed, 242 insertions(+), 2 deletions(-)
>>
>> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc b/Platform/Hisilicon/HiKey960/HiKey960.dsc
>> index 98289c0..a6864c1 100644
>> --- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
>> +++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
>> @@ -78,6 +78,9 @@
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE
>>   
>>   [PcdsFixedAtBuild.common]
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0x1AD88048
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x7FB8
>> +
>>     gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
>>   
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Alpha"
>> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf b/Platform/Hisilicon/HiKey960/HiKey960.fdf
>> index 655032a..af10430 100644
>> --- a/Platform/Hisilicon/HiKey960/HiKey960.fdf
>> +++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf
>> @@ -26,12 +26,12 @@
>>   
>>   [FD.BL33_AP_UEFI]
>>   BaseAddress   = 0x1AC98000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
>> -Size          = 0x000F0000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
>> +Size          = 0x00100000|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     = 0x100
>>   
>>   ################################################################################
>>   #
>> @@ -53,6 +53,243 @@ NumBlocks     = 0xF0
>>   gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>>   FV = FVMAIN_COMPACT
>>   
>> +0x000F0000|0x00008000
>> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>> +DATA = {
>> +  ## This is the EFI_FIRMWARE_VOLUME_HEADER
>> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +  # FileSystemGuid: gEfiSystemNvDataFvGuid         =
>> +  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
>> +  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
>> +  # FvLength: 0x8000
>> +  0x00, 0x80, 0x0, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +  #Signature "_FVH"       #Attributes
>> +  0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
>> +  #HeaderLength #CheckSum #ExtHeaderOffset #Reserved #Revision
>> +  0x48, 0x00, 0x36, 0x09, 0x00, 0x00, 0x00, 0x02,
>> +  #Blockmap[0]: 8 Blocks * 0x1000 Bytes / Block
>> +  0x08, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
>> +  #Blockmap[1]: End
>> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +  ## end of EFI_FIRMWARE_VOLUME_HEADER.
>> +
>> +  ### Offset (0x48) ###
>> +  ## This is the VARIABLE_STORE_HEADER gEfiVariableGuid
>> +  0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
>> +  0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
>> +  #Size: 0x8000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x7FB8
>> +  0xB8, 0x7F, 0x00, 0x00,
>> +  #FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
>> +  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +
>> +  ### Offset (0x64) ###
>> +  ## This is the VARIABLE_HEADER
>> +  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
>> +  0xAA, 0x55, 0x3F, 0x00,
>> +  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
>> +  0x03, 0x00, 0x00, 0x00,
>> +  #NameSize:
>> +  0x14, 0x00, 0x00, 0x00,
>> +  #DataSize:
>> +  0x18, 0x00, 0x00, 0x00,
>> +  #VariableGuid: gEfiGlobalVariableGuid
>> +  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
>> +  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
>> +  ## End of VARIABLE_HEADER. Offset (0x84)
>> +  #VariableName: BootOrder
>> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
>> +  0x4F, 0x00, 0x72, 0x00, 0x64, 0x00, 0x65, 0x00,
>> +  0x72, 0x00, 0x00, 0x00,
>> +  #Data
>> +  0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x03, 0x00,
>> +  0x04, 0x00, 0x05, 0x00, 0x06, 0x00, 0x07, 0x00,
>> +  0x08, 0x00, 0x09, 0x00, 0x0a, 0x00, 0x0b, 0x00,
>> +  ### End of BootOrder.
>> +
>> +  ### Offset (0xB0) ###
>> +  ## This is the VARIABLE_HEADER
>> +  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
>> +  0xAA, 0x55, 0x3F, 0x00,
>> +  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
>> +  0x03, 0x00, 0x00, 0x00,
>> +  #NameSize:
>> +  0x12, 0x00, 0x00, 0x00,
>> +  #DataSize:
>> +  0x42, 0x00, 0x00, 0x00,
>> +  #VariableGuid: gEfiGlobalVariableGuid
>> +  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
>> +  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
>> +  #VariableName: Boot0000
>> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
>> +  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
>> +  0x00, 0x00,
>> +  ###  Offset (0xE2), Size(0x32) ###
>> +  #Data:
>> +  #Attributes: LOAD_OPTION_ACTIVE
>> +  0x01, 0x00, 0x00, 0x00,
>> +  #FilePathListLength
>> +  0x26, 0x00,
>> +  #Description: "Boot on SD"
>> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #Boot
>> +  0x20, 0x00, 0x6F, 0x00, 0x6E, 0x00, 0x20, 0x00,      # on
>> +  0x53, 0x00, 0x44, 0x00, 0x00, 0x00,                  #SD
>> +  #FilePathList (0x26)
>> +  0x01, 0x04, 0x1D, 0x00, 0x5B, 0x90, 0x51, 0x0D,
>> +  0x7E, 0xB7, 0x2A, 0x45, 0xA2, 0xC0, 0xEC, 0xA0,
>> +  0xCC, 0x8D, 0x51, 0x4A, 0x00, 0xF0, 0x37, 0xFF,
>> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x1A, 0x05,
>> +  0x00, 0x00, 0x7F, 0xFF, 0x04, 0x00,
>> +  ### Offset (0x124), Size (0x42) ###
>> +  ### End of Boot0000
>> +
>>
> 
> While in both ArmVirtPkg and OvmfPkg we use a similar DATA definition
> for *formatting* the pristine variable store (just setting the
> absolutely necessary headers via a hexdump), I think that adding
> "business content" like this is a bad idea. For one, if the defaults
> have to be updated ever again, the patches for the DATA definitions will
> look terrible. It's hard to review and maintain hexdump like these; we
> should keep such DATA definitions to the absolute minimum.
> 
> Such "default" boot options should be set by the platform's
> PlatformBootManagerLib instance. PlatformBootManagerLib is responsible
> for generating boot options. In doing that, PlatformBootManagerLib can
> call helper functions from UefiBootManagerLib, so everything need not be
> done manually.
> 
> For example, EfiBootManagerGetLoadOptions() can be used to retrieve the
> current boot options. Then, you can iterate over the four boot options
> that you want to ensure exist -- for each:
> 
> - create a EFI_BOOT_MANAGER_LOAD_OPTION object with
> EfiBootManagerInitializeLoadOption(),
> - check if the option already exists, with EfiBootManagerFindLoadOption(),
> - if the option doesn't exist yet, add it with
> EfiBootManagerAddLoadOptionVariable(),
> - free the EFI_BOOT_MANAGER_LOAD_OPTION object with
> EfiBootManagerFreeLoadOption()
> 
> Finally, free the originally retrieved set of boot options with
> EfiBootManagerFreeLoadOptions().
> 
> You can find example code in
> "ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c".
> 
> Thanks,
> Laszlo
> 

Hi Laszlo,

Yes, it's a bit hard to review and maintain. Actually, I implemented a 
PlatformBootManagerLib in this link 
(https://github.com/96boards-hikey/OpenPlatformPkg/blob/testing/hikey960_v1.3.4/Platforms/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c).

I format it with hard code since I want to re-use 
PlatformBootManagerLib in ArmPkg. Because these boot options only exist 
on HiKey/HiKey960 platforms, and most of them are same of the one in 
ArmPkg. If I append them into ArmPkg, it's not common enough. At last, I 
hope to implement a non-volatile region on flash devices in the future. 
It means that implementing a HiKey specific PlatformBootManagerLib is 
only a temporary solution.

It seems that I have two options to go ahead.
1. Move those hard code boot options into a FV file, and put the FV file
in edk2-non-osi repository.

2. Implement a HiKey related PlatformBootManagerLib.

Which one do you prefer?

Best Regards
Haojian


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

* Re: [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options
  2018-02-22  8:57     ` Haojian Zhuang
@ 2018-02-22 10:56       ` Laszlo Ersek
  2018-02-22 11:32         ` Leif Lindholm
  2018-02-22 11:49         ` Haojian Zhuang
  0 siblings, 2 replies; 8+ messages in thread
From: Laszlo Ersek @ 2018-02-22 10:56 UTC (permalink / raw)
  To: haojian.zhuang@linaro.org
  Cc: edk2-devel@lists.01.org, leif.lindholm@linaro.org,
	ard.biesheuvel@linaro.org, linaro-uefi@lists.linaro.org

On 02/22/18 09:57, Haojian Zhuang wrote:
> On 02/20/2018 11:54 PM, Laszlo Ersek wrote:
>> Hi,
>>
>> On 02/15/18 16:14, Haojian Zhuang wrote:
>>> Add four boot options in emu variable region. They are
>>> "Boot on SD", "Grub", "Android Boot" and "Android Fastboot".
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>>> ---
>>>   Platform/Hisilicon/HiKey960/HiKey960.dsc |   3 +
>>>   Platform/Hisilicon/HiKey960/HiKey960.fdf | 241 ++++++++++++++++++++++++++++++-
>>>   2 files changed, 242 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.dsc b/Platform/Hisilicon/HiKey960/HiKey960.dsc
>>> index 98289c0..a6864c1 100644
>>> --- a/Platform/Hisilicon/HiKey960/HiKey960.dsc
>>> +++ b/Platform/Hisilicon/HiKey960/HiKey960.dsc
>>> @@ -78,6 +78,9 @@
>>>     gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|FALSE
>>>   
>>>   [PcdsFixedAtBuild.common]
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0x1AD88048
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x7FB8
>>> +
>>>     gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
>>>   
>>>     gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Alpha"
>>> diff --git a/Platform/Hisilicon/HiKey960/HiKey960.fdf b/Platform/Hisilicon/HiKey960/HiKey960.fdf
>>> index 655032a..af10430 100644
>>> --- a/Platform/Hisilicon/HiKey960/HiKey960.fdf
>>> +++ b/Platform/Hisilicon/HiKey960/HiKey960.fdf
>>> @@ -26,12 +26,12 @@
>>>   
>>>   [FD.BL33_AP_UEFI]
>>>   BaseAddress   = 0x1AC98000|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
>>> -Size          = 0x000F0000|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
>>> +Size          = 0x00100000|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     = 0x100
>>>   
>>>   ################################################################################
>>>   #
>>> @@ -53,6 +53,243 @@ NumBlocks     = 0xF0
>>>   gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>>>   FV = FVMAIN_COMPACT
>>>   
>>> +0x000F0000|0x00008000
>>> +gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>>> +DATA = {
>>> +  ## This is the EFI_FIRMWARE_VOLUME_HEADER
>>> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> +  # FileSystemGuid: gEfiSystemNvDataFvGuid         =
>>> +  0x8D, 0x2B, 0xF1, 0xFF, 0x96, 0x76, 0x8B, 0x4C,
>>> +  0xA9, 0x85, 0x27, 0x47, 0x07, 0x5B, 0x4F, 0x50,
>>> +  # FvLength: 0x8000
>>> +  0x00, 0x80, 0x0, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> +  #Signature "_FVH"       #Attributes
>>> +  0x5f, 0x46, 0x56, 0x48, 0xff, 0xfe, 0x04, 0x00,
>>> +  #HeaderLength #CheckSum #ExtHeaderOffset #Reserved #Revision
>>> +  0x48, 0x00, 0x36, 0x09, 0x00, 0x00, 0x00, 0x02,
>>> +  #Blockmap[0]: 8 Blocks * 0x1000 Bytes / Block
>>> +  0x08, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
>>> +  #Blockmap[1]: End
>>> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> +  ## end of EFI_FIRMWARE_VOLUME_HEADER.
>>> +
>>> +  ### Offset (0x48) ###
>>> +  ## This is the VARIABLE_STORE_HEADER gEfiVariableGuid
>>> +  0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
>>> +  0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
>>> +  #Size: 0x8000 (gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize) - 0x48 (size of EFI_FIRMWARE_VOLUME_HEADER) = 0x7FB8
>>> +  0xB8, 0x7F, 0x00, 0x00,
>>> +  #FORMATTED: 0x5A #HEALTHY: 0xFE #Reserved: UINT16 #Reserved1: UINT32
>>> +  0x5A, 0xFE, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>>> +
>>> +  ### Offset (0x64) ###
>>> +  ## This is the VARIABLE_HEADER
>>> +  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
>>> +  0xAA, 0x55, 0x3F, 0x00,
>>> +  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
>>> +  0x03, 0x00, 0x00, 0x00,
>>> +  #NameSize:
>>> +  0x14, 0x00, 0x00, 0x00,
>>> +  #DataSize:
>>> +  0x18, 0x00, 0x00, 0x00,
>>> +  #VariableGuid: gEfiGlobalVariableGuid
>>> +  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
>>> +  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
>>> +  ## End of VARIABLE_HEADER. Offset (0x84)
>>> +  #VariableName: BootOrder
>>> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
>>> +  0x4F, 0x00, 0x72, 0x00, 0x64, 0x00, 0x65, 0x00,
>>> +  0x72, 0x00, 0x00, 0x00,
>>> +  #Data
>>> +  0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x03, 0x00,
>>> +  0x04, 0x00, 0x05, 0x00, 0x06, 0x00, 0x07, 0x00,
>>> +  0x08, 0x00, 0x09, 0x00, 0x0a, 0x00, 0x0b, 0x00,
>>> +  ### End of BootOrder.
>>> +
>>> +  ### Offset (0xB0) ###
>>> +  ## This is the VARIABLE_HEADER
>>> +  #StartId: VARIABLE_DATA #State: VAR_ADDED #Reserved
>>> +  0xAA, 0x55, 0x3F, 0x00,
>>> +  #Attributes: EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS
>>> +  0x03, 0x00, 0x00, 0x00,
>>> +  #NameSize:
>>> +  0x12, 0x00, 0x00, 0x00,
>>> +  #DataSize:
>>> +  0x42, 0x00, 0x00, 0x00,
>>> +  #VariableGuid: gEfiGlobalVariableGuid
>>> +  0x61, 0xDF, 0xE4, 0x8B, 0xCA, 0x93, 0xD2, 0x11,
>>> +  0xAA, 0x0D, 0x00, 0xE0, 0x98, 0x03, 0x2B, 0x8C,
>>> +  #VariableName: Boot0000
>>> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,
>>> +  0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
>>> +  0x00, 0x00,
>>> +  ###  Offset (0xE2), Size(0x32) ###
>>> +  #Data:
>>> +  #Attributes: LOAD_OPTION_ACTIVE
>>> +  0x01, 0x00, 0x00, 0x00,
>>> +  #FilePathListLength
>>> +  0x26, 0x00,
>>> +  #Description: "Boot on SD"
>>> +  0x42, 0x00, 0x6F, 0x00, 0x6F, 0x00, 0x74, 0x00,      #Boot
>>> +  0x20, 0x00, 0x6F, 0x00, 0x6E, 0x00, 0x20, 0x00,      # on
>>> +  0x53, 0x00, 0x44, 0x00, 0x00, 0x00,                  #SD
>>> +  #FilePathList (0x26)
>>> +  0x01, 0x04, 0x1D, 0x00, 0x5B, 0x90, 0x51, 0x0D,
>>> +  0x7E, 0xB7, 0x2A, 0x45, 0xA2, 0xC0, 0xEC, 0xA0,
>>> +  0xCC, 0x8D, 0x51, 0x4A, 0x00, 0xF0, 0x37, 0xFF,
>>> +  0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x1A, 0x05,
>>> +  0x00, 0x00, 0x7F, 0xFF, 0x04, 0x00,
>>> +  ### Offset (0x124), Size (0x42) ###
>>> +  ### End of Boot0000
>>> +
>>>
>>
>> While in both ArmVirtPkg and OvmfPkg we use a similar DATA definition
>> for *formatting* the pristine variable store (just setting the
>> absolutely necessary headers via a hexdump), I think that adding
>> "business content" like this is a bad idea. For one, if the defaults
>> have to be updated ever again, the patches for the DATA definitions will
>> look terrible. It's hard to review and maintain hexdump like these; we
>> should keep such DATA definitions to the absolute minimum.
>>
>> Such "default" boot options should be set by the platform's
>> PlatformBootManagerLib instance. PlatformBootManagerLib is responsible
>> for generating boot options. In doing that, PlatformBootManagerLib can
>> call helper functions from UefiBootManagerLib, so everything need not be
>> done manually.
>>
>> For example, EfiBootManagerGetLoadOptions() can be used to retrieve the
>> current boot options. Then, you can iterate over the four boot options
>> that you want to ensure exist -- for each:
>>
>> - create a EFI_BOOT_MANAGER_LOAD_OPTION object with
>> EfiBootManagerInitializeLoadOption(),
>> - check if the option already exists, with EfiBootManagerFindLoadOption(),
>> - if the option doesn't exist yet, add it with
>> EfiBootManagerAddLoadOptionVariable(),
>> - free the EFI_BOOT_MANAGER_LOAD_OPTION object with
>> EfiBootManagerFreeLoadOption()
>>
>> Finally, free the originally retrieved set of boot options with
>> EfiBootManagerFreeLoadOptions().
>>
>> You can find example code in
>> "ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c".
>>
>> Thanks,
>> Laszlo
>>
> 
> Hi Laszlo,
> 
> Yes, it's a bit hard to review and maintain. Actually, I implemented a 
> PlatformBootManagerLib in this link 
> (https://github.com/96boards-hikey/OpenPlatformPkg/blob/testing/hikey960_v1.3.4/Platforms/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c).
> 
> I format it with hard code since I want to re-use 
> PlatformBootManagerLib in ArmPkg. Because these boot options only exist 
> on HiKey/HiKey960 platforms, and most of them are same of the one in 
> ArmPkg. If I append them into ArmPkg, it's not common enough. At last, I 
> hope to implement a non-volatile region on flash devices in the future. 
> It means that implementing a HiKey specific PlatformBootManagerLib is 
> only a temporary solution.

Sorry, I don't understand how this follows. Again: why is it only a
temporary solution to implement a PlatformBootManagerLib instance for HiKey?

The library class says "Platform" in the name. Platforms are supposed to
provide it, one way or another.

If you want to minimize code duplication between ArmPkg and HiKey, it
should be possible to factor out another library class from ArmPkg's
PlatformBootManagerLib instance. It could be a function that returns the
list of platform-specific boot options -- as an array of
EFI_BOOT_MANAGER_LOAD_OPTION elements -- that should always exist. Then
ArmPkg's PlatformBootManagerLib would perform the iteration that I
described above.

Platforms different from HiKey would return an empty array, while HiKey
could return the four options it needs.

Another benefit is that these four boot options would be restored on
every boot, even if the user deleted them. This seems independent of
whether the HiKey platform has functional non-volatile storage or not.

> It seems that I have two options to go ahead.
> 1. Move those hard code boot options into a FV file, and put the FV file
> in edk2-non-osi repository.
> 
> 2. Implement a HiKey related PlatformBootManagerLib.

I think option #2 is far superior. You don't have to duplicate all code
from ArmPkg's PlatformBootManagerLib for that, you can extract another
utility library class / callback function just for providing the options
you always want.

Or perhaps you *don't* want them always, just until NVRAM support
arrives to HiKey? And, in that case, it will be alright for the user to
delete these options permanently?

... I still think extracting a new lib class for these default boot
options would be better. If at some point you'd like to drop the default
boot options, it's easy to switch the new callback to returning zero
elements. The lib class can remain useful to other development platforms.

Thanks
Laszlo


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

* Re: [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options
  2018-02-22 10:56       ` Laszlo Ersek
@ 2018-02-22 11:32         ` Leif Lindholm
  2018-02-22 11:49         ` Haojian Zhuang
  1 sibling, 0 replies; 8+ messages in thread
From: Leif Lindholm @ 2018-02-22 11:32 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: haojian.zhuang@linaro.org, edk2-devel@lists.01.org,
	ard.biesheuvel@linaro.org, linaro-uefi@lists.linaro.org

On Thu, Feb 22, 2018 at 11:56:22AM +0100, Laszlo Ersek wrote:
> Sorry, I don't understand how this follows. Again: why is it only a
> temporary solution to implement a PlatformBootManagerLib instance for HiKey?
> 
> The library class says "Platform" in the name. Platforms are supposed to
> provide it, one way or another.

This is probably triggered by me. I have been pushing people to stay
away from duplicating the generic PlatformBootManagerLib unless they
really need to.

> If you want to minimize code duplication between ArmPkg and HiKey, it
> should be possible to factor out another library class from ArmPkg's
> PlatformBootManagerLib instance. It could be a function that returns the
> list of platform-specific boot options -- as an array of
> EFI_BOOT_MANAGER_LOAD_OPTION elements -- that should always exist. Then
> ArmPkg's PlatformBootManagerLib would perform the iteration that I
> described above.

This sounds like an excellent idea.

/
    Leif


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

* Re: [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options
  2018-02-22 10:56       ` Laszlo Ersek
  2018-02-22 11:32         ` Leif Lindholm
@ 2018-02-22 11:49         ` Haojian Zhuang
  1 sibling, 0 replies; 8+ messages in thread
From: Haojian Zhuang @ 2018-02-22 11:49 UTC (permalink / raw)
  To: lersek@redhat.com
  Cc: edk2-devel@lists.01.org, leif.lindholm@linaro.org,
	ard.biesheuvel@linaro.org, linaro-uefi@lists.linaro.org

On 02/22/2018 06:56 PM, Laszlo Ersek wrote:
> On 02/22/18 09:57, Haojian Zhuang wrote:
>> On 02/20/2018 11:54 PM, Laszlo Ersek wrote:
>>> Hi,
>>>
>>> On 02/15/18 16:14, Haojian Zhuang wrote:
>>>> Add four boot options in emu variable region. They are
>>>> "Boot on SD", "Grub", "Android Boot" and "Android Fastboot".
>>>>
>>>
>>> While in both ArmVirtPkg and OvmfPkg we use a similar DATA definition
>>> for *formatting* the pristine variable store (just setting the
>>> absolutely necessary headers via a hexdump), I think that adding
>>> "business content" like this is a bad idea. For one, if the defaults
>>> have to be updated ever again, the patches for the DATA definitions will
>>> look terrible. It's hard to review and maintain hexdump like these; we
>>> should keep such DATA definitions to the absolute minimum.
>>>
>>> Such "default" boot options should be set by the platform's
>>> PlatformBootManagerLib instance. PlatformBootManagerLib is responsible
>>> for generating boot options. In doing that, PlatformBootManagerLib can
>>> call helper functions from UefiBootManagerLib, so everything need not be
>>> done manually.
>>>
>>> For example, EfiBootManagerGetLoadOptions() can be used to retrieve the
>>> current boot options. Then, you can iterate over the four boot options
>>> that you want to ensure exist -- for each:
>>>
>>> - create a EFI_BOOT_MANAGER_LOAD_OPTION object with
>>> EfiBootManagerInitializeLoadOption(),
>>> - check if the option already exists, with EfiBootManagerFindLoadOption(),
>>> - if the option doesn't exist yet, add it with
>>> EfiBootManagerAddLoadOptionVariable(),
>>> - free the EFI_BOOT_MANAGER_LOAD_OPTION object with
>>> EfiBootManagerFreeLoadOption()
>>>
>>> Finally, free the originally retrieved set of boot options with
>>> EfiBootManagerFreeLoadOptions().
>>>
>>> You can find example code in
>>> "ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c".
>>>
>>> Thanks,
>>> Laszlo
>>>
>>
>> Hi Laszlo,
>>
>> Yes, it's a bit hard to review and maintain. Actually, I implemented a
>> PlatformBootManagerLib in this link
>> (https://github.com/96boards-hikey/OpenPlatformPkg/blob/testing/hikey960_v1.3.4/Platforms/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c).
>>
>> I format it with hard code since I want to re-use
>> PlatformBootManagerLib in ArmPkg. Because these boot options only exist
>> on HiKey/HiKey960 platforms, and most of them are same of the one in
>> ArmPkg. If I append them into ArmPkg, it's not common enough. At last, I
>> hope to implement a non-volatile region on flash devices in the future.
>> It means that implementing a HiKey specific PlatformBootManagerLib is
>> only a temporary solution.
> 
> Sorry, I don't understand how this follows. Again: why is it only a
> temporary solution to implement a PlatformBootManagerLib instance for HiKey?
> 
> The library class says "Platform" in the name. Platforms are supposed to
> provide it, one way or another.
> 
> If you want to minimize code duplication between ArmPkg and HiKey, it
> should be possible to factor out another library class from ArmPkg's
> PlatformBootManagerLib instance. It could be a function that returns the
> list of platform-specific boot options -- as an array of
> EFI_BOOT_MANAGER_LOAD_OPTION elements -- that should always exist. Then
> ArmPkg's PlatformBootManagerLib would perform the iteration that I
> described above.
> 
> Platforms different from HiKey would return an empty array, while HiKey
> could return the four options it needs.
> 
> Another benefit is that these four boot options would be restored on
> every boot, even if the user deleted them. This seems independent of
> whether the HiKey platform has functional non-volatile storage or not.
> 
>> It seems that I have two options to go ahead.
>> 1. Move those hard code boot options into a FV file, and put the FV file
>> in edk2-non-osi repository.
>>
>> 2. Implement a HiKey related PlatformBootManagerLib.
> 
> I think option #2 is far superior. You don't have to duplicate all code
> from ArmPkg's PlatformBootManagerLib for that, you can extract another
> utility library class / callback function just for providing the options
> you always want.
> 
> Or perhaps you *don't* want them always, just until NVRAM support
> arrives to HiKey? And, in that case, it will be alright for the user to
> delete these options permanently?

Yes, I hope the default boot options could be dropped when NVRAM support 
arrives on HiKey. User could decide which boot option is really good for 
him.

> 
> ... I still think extracting a new lib class for these default boot
> options would be better. If at some point you'd like to drop the default
> boot options, it's easy to switch the new callback to returning zero
> elements. The lib class can remain useful to other development platforms.
>

OK. I'll try to add the new callback in it.

Best Regards
Haojian


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

end of thread, other threads:[~2018-02-22 11:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-15 15:14 [PATCH 0/2] add boot options in HiKey Haojian Zhuang
2018-02-15 15:14 ` [PATCH 1/2] Platform/Hisilicon/HiKey960: add boot options Haojian Zhuang
2018-02-20 15:54   ` Laszlo Ersek
2018-02-22  8:57     ` Haojian Zhuang
2018-02-22 10:56       ` Laszlo Ersek
2018-02-22 11:32         ` Leif Lindholm
2018-02-22 11:49         ` Haojian Zhuang
2018-02-15 15:14 ` [PATCH 2/2] Platform/Hisilicon/HiKey: " Haojian Zhuang

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