Re: [Linaro-uefi] [linaro-uefi] Hisilicon D03/D05: set boot option by BMC

On Thu, May 18, 2017 at 10:12:03AM +0800, Chenhui Sun wrote:

From: Chenhui Sun chenhui.sun@linaro.com

Support the feature that BIOS get boot option from BMC and put it in the first boot order.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: huangming huangming23@huawei.com Signed-off-by: Heyi Guo guoheyi@huawei.com Signed-off-by: Yi Li Phoenix.liyi@huawei.com Signed-off-by: Chenhui Sun sunchenhui@huawei.com

---

Chips/Hisilicon/Include/Library/IpmiCmdLib.h | 94 +++++ .../Library/PlatformIntelBdsLib/IntelBdsPlatform.c | 423 +++++++++++++++++++++ .../PlatformIntelBdsLib/PlatformIntelBdsLib.inf | 4 + 3 files changed, 521 insertions(+) create mode 100644 Chips/Hisilicon/Include/Library/IpmiCmdLib.h

diff --git a/Chips/Hisilicon/Include/Library/IpmiCmdLib.h b/Chips/Hisilicon/Include/Library/IpmiCmdLib.h new file mode 100644 index 0000000..8868b76 --- /dev/null +++ b/Chips/Hisilicon/Include/Library/IpmiCmdLib.h @@ -0,0 +1,94 @@ +/** @file +* +* Copyright (c) 2017, Hisilicon Limited. All rights reserved. +* Copyright (c) 2017, Linaro 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. +* +**/

+#ifndef _IPMI_CMD_LIB_H_ +#define _IPMI_CMD_LIB_H_

+#define BOOT_OPTION_BOOT_FLAG_VALID 1 +#define BOOT_OPTION_BOOT_FLAG_INVALID 0

+typedef enum {

• NoOverride = 0x0,
• ForcePxe,
• ForceDefaultHardDisk,
• ForceDefaultHardDiskSafeMode,
• ForceDefaultDiagnosticPartition,
• ForceDefaultCD,
• ForceSetupUtility,
• ForceRemoteRemovableMedia,
• ForceRemoteCD,
• ForcePrimaryRemoteMedia,
• ForceRemoteHardDisk = 0xB,
• ForcePrimaryRemovableMedia = 0xF

+} BOOT_DEVICE_SELECTOR;

+// +// Get System Boot Option data structure +// +typedef struct {

• UINT8 ParameterVersion :4;
• UINT8 Reserved1 :4;
• UINT8 ParameterSelector :7;
• UINT8 ParameterValid :1;
• //
• // Boot Flags Data 1
• //
• UINT8 Reserved2 :5;
• UINT8 BiosBootType :1;
• UINT8 Persistent :1;
• UINT8 BootFlagsValid :1;
• //
• // Boot Flags Data 2
• //
• UINT8 LockResetBtn :1;
• UINT8 ScreenBlank :1;
• UINT8 BootDeviceSelector :4;
• UINT8 LockKeyboard :1;
• UINT8 ClearCmos :1;
• //
• // Boot Flags Data 3
• //
• UINT8 ConsoleRedirectionControl :2;
• UINT8 LockSleepBtn :1;
• UINT8 UserPasswordByPass :1;
• UINT8 Reserved3 :1;
• UINT8 FirmwareVerbosity :2;
• UINT8 LockPowerBtn :1;
• //
• // Boot Flags Data 4
• //
• UINT8 MuxControlOverride :3;
• UINT8 ShareModeOverride :1;
• UINT8 Reserved4 :4;
• //
• // Boot Flags Data 5
• //
• UINT8 DeviceInstanceSelector :5;
• UINT8 Reserved5 :3;

+} IPMI_GET_BOOT_OPTION;

+EFI_STATUS +EFIAPI +IpmiCmdSetSysBootOptions (

• OUT IPMI_GET_BOOT_OPTION *BootOption
• );

+EFI_STATUS +EFIAPI +IpmiCmdGetSysBootOptions (

• IN IPMI_GET_BOOT_OPTION *BootOption
• );

+#endif diff --git a/Chips/Hisilicon/Library/PlatformIntelBdsLib/IntelBdsPlatform.c b/Chips/Hisilicon/Library/PlatformIntelBdsLib/IntelBdsPlatform.c index efefeb6..78703f1 100644 --- a/Chips/Hisilicon/Library/PlatformIntelBdsLib/IntelBdsPlatform.c +++ b/Chips/Hisilicon/Library/PlatformIntelBdsLib/IntelBdsPlatform.c @@ -21,16 +21,23 @@ #include <IndustryStandard/Pci22.h> #include <Library/DevicePathLib.h> +#include <Library/GenericBdsLib.h> +#include <Library/IpmiCmdLib.h> #include <Library/PcdLib.h> #include <Library/PlatformBdsLib.h> +#include <Library/PrintLib.h> #include <Library/UefiLib.h> #include <Protocol/DevicePath.h> +#include <Protocol/DevicePathToText.h> #include <Protocol/GraphicsOutput.h> #include <Protocol/PciIo.h> #include <Protocol/PciRootBridgeIo.h> +#include <Guid/GlobalVariable.h> #include "IntelBdsPlatform.h" +GUID gOemBootVaraibleGuid = {0xb7784577, 0x5aaf, 0x4557, {0xa1, 0x99,

• 0xd4, 0xa4, 0x2f, 0x45, 0x06, 0xf8} };

//3CEF354A-3B7A-4519-AD70-72A134698311 GUID gEblFileGuid = {0x3CEF354A, 0x3B7A, 0x4519, {0xAD, 0x70, @@ -142,6 +149,418 @@ STATIC PLATFORM_USB_KEYBOARD mUsbKeyboard = { } }; +STATIC +UINT16 +GetBBSTypeFromFileSysPath (

• IN CHAR16 *UsbPathTxt,
• IN CHAR16 *FileSysPathTxt,
• IN EFI_DEVICE_PATH_PROTOCOL *FileSysPath
• )

+{

• EFI_DEVICE_PATH_PROTOCOL *Node;
• if (StrnCmp (UsbPathTxt, FileSysPathTxt, StrLen (UsbPathTxt)) == 0) {
• Node = FileSysPath;
• while (!IsDevicePathEnd (Node)) {

•  if ((DevicePathType (Node) == MEDIA_DEVICE_PATH) &&
  

•      (DevicePathSubType (Node) == MEDIA_CDROM_DP)) {
  

•    return BBS_TYPE_CDROM;
  

•  }
  

•  Node = NextDevicePathNode (Node);
  

• }
• }
• return BBS_TYPE_UNKNOWN;

+}

+STATIC +UINT16 +GetBBSTypeFromUsbPath (

• IN CONST EFI_DEVICE_PATH_PROTOCOL *UsbPath
• )

+{

• EFI_STATUS Status;
• EFI_HANDLE *FileSystemHandles;
• UINTN NumberFileSystemHandles;
• UINTN Index;
• EFI_DEVICE_PATH_PROTOCOL *FileSysPath;
• EFI_DEVICE_PATH_TO_TEXT_PROTOCOL *DevPathToText;
• CHAR16 *UsbPathTxt;
• CHAR16 *FileSysPathTxt;
• UINT16 Result;
• Status = gBS->LocateProtocol (&gEfiDevicePathToTextProtocolGuid, NULL, (VOID **) &DevPathToText);
• if (EFI_ERROR (Status)) {
• DEBUG ((DEBUG_ERROR, "Locate DevicePathToTextPro %r\n", Status));
• return BBS_TYPE_UNKNOWN;
• }
• Result = BBS_TYPE_UNKNOWN;
• UsbPathTxt = DevPathToText->ConvertDevicePathToText (UsbPath, TRUE, TRUE);
• if (UsbPathTxt == NULL) {
• return Result;
• }
• Status = gBS->LocateHandleBuffer (

•              ByProtocol,
  

•              &gEfiSimpleFileSystemProtocolGuid,
  

•              NULL,
  

•              &NumberFileSystemHandles,
  

•              &FileSystemHandles
  

•              );
  

• if (EFI_ERROR (Status)) {
• DEBUG ((DEBUG_ERROR, "Locate SimpleFileSystemProtocol error(%r)\n", Status));
• FreePool (UsbPathTxt);
• return BBS_TYPE_UNKNOWN;
• }
• for (Index = 0; Index < NumberFileSystemHandles; Index++) {
• FileSysPath = DevicePathFromHandle (FileSystemHandles[Index]);
• FileSysPathTxt = DevPathToText->ConvertDevicePathToText (FileSysPath, TRUE, TRUE);
• if (FileSysPathTxt == NULL) {

•  continue;
  

• }
• Result = GetBBSTypeFromFileSysPath (UsbPathTxt, FileSysPathTxt, FileSysPath);
• FreePool (FileSysPathTxt);
• if (Result != BBS_TYPE_UNKNOWN) {

•  break;
  

• }
• }
• if (NumberFileSystemHandles != 0) {
• FreePool (FileSystemHandles);
• }
• FreePool (UsbPathTxt);
• return Result;

+}

+STATIC +UINT16 +GetBBSTypeFromMessagingDevicePath (

• IN EFI_DEVICE_PATH_PROTOCOL *DevicePath,
• IN EFI_DEVICE_PATH_PROTOCOL *Node
• )

+{

• VENDOR_DEVICE_PATH *Vendor;
• UINT16 Result;
• Result = BBS_TYPE_UNKNOWN;
• switch (DevicePathSubType (Node)) {
• case MSG_MAC_ADDR_DP:
• Result = BBS_TYPE_EMBEDDED_NETWORK;
• break;
• case MSG_USB_DP:
• Result = GetBBSTypeFromUsbPath (DevicePath);
• if (Result == BBS_TYPE_UNKNOWN) {

•  Result =  BBS_TYPE_USB;
  

• }
• break;
• case MSG_SATA_DP:
• Result = BBS_TYPE_HARDDRIVE;
• break;
• case MSG_VENDOR_DP:
• Vendor = (VENDOR_DEVICE_PATH *) (Node);
• if ((&Vendor->Guid) != NULL) {

•  if (CompareGuid (&Vendor->Guid, &((EFI_GUID) DEVICE_PATH_MESSAGING_SAS))) {
  

•    Result = BBS_TYPE_HARDDRIVE;
  

•  }
  

• }
• break;
• default:
• Result = BBS_TYPE_UNKNOWN;
• break;
• }
• return Result;

+}

+STATIC +UINT16 +GetBBSTypeByDevicePath (

• IN EFI_DEVICE_PATH_PROTOCOL *DevicePath
• )

+{

• EFI_DEVICE_PATH_PROTOCOL *Node;
• UINT16 Result;
• Result = BBS_TYPE_UNKNOWN;
• if (DevicePath == NULL) {
• return Result;
• }
• Node = DevicePath;
• while (!IsDevicePathEnd (Node)) {
• switch (DevicePathType (Node)) {
• case MEDIA_DEVICE_PATH:

•  if (DevicePathSubType (Node) == MEDIA_CDROM_DP) {
  

•    Result = BBS_TYPE_CDROM;
  

•  }
  

•  break;
  

• case MESSAGING_DEVICE_PATH:

•  Result = GetBBSTypeFromMessagingDevicePath (DevicePath, Node);
  

•  break;
  

• default:

•  Result = BBS_TYPE_UNKNOWN;
  

•  break;
  

• }
• if (Result != BBS_TYPE_UNKNOWN) {

•  break;
  

• }
• Node = NextDevicePathNode (Node);
• }
• return Result;

+}

+STATIC +EFI_STATUS +GetBmcBootOptionsSetting (

• OUT IPMI_GET_BOOT_OPTION *BmcBootOpt
• )

+{

• EFI_STATUS Status;
• Status = IpmiCmdGetSysBootOptions (BmcBootOpt);
• if (EFI_ERROR (Status)) {
• DEBUG ((DEBUG_ERROR, "Get iBMC BootOpts %r!\n", Status));
• return Status;
• }
• if (BmcBootOpt->BootFlagsValid != BOOT_OPTION_BOOT_FLAG_VALID) {
• return EFI_NOT_FOUND;
• }
• if (BmcBootOpt->Persistent) {
• BmcBootOpt->BootFlagsValid = BOOT_OPTION_BOOT_FLAG_VALID;
• } else {
• BmcBootOpt->BootFlagsValid = BOOT_OPTION_BOOT_FLAG_INVALID;
• }
• Status = IpmiCmdSetSysBootOptions (BmcBootOpt);
• if (EFI_ERROR (Status)) {
• DEBUG ((DEBUG_ERROR, "Set iBMC BootOpts %r!\n", Status));
• }
• return Status;

+}

+STATIC +VOID +RestoreBootOrder (

• VOID
• )

+{

• EFI_STATUS Status;
• UINT16 *BootOrder;
• UINTN BootOrderSize;
• GetVariable2 (L"BootOrderBak", &gOemBootVaraibleGuid, (VOID **) &BootOrder, &BootOrderSize);
• if (BootOrder == NULL) {
• return ;
• }
• Print(L"Restore BootOrder(%d).\n", BootOrderSize / sizeof (UINT16));
• Status = gRT->SetVariable (

•              L"BootOrder",
  

•              &gEfiGlobalVariableGuid,
  

•              EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
  

•              BootOrderSize,
  

•              BootOrder
  

•              );
  

• if (EFI_ERROR (Status)) {
• DEBUG ((DEBUG_ERROR, "SetVariable BootOrder %r!\n", Status));
• }
• Status = gRT->SetVariable (

•              L"BootOrderBak",
  

•              &gOemBootVaraibleGuid,
  

•              EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
  

•              0,
  

•              NULL
  

•              );
  

• if (EFI_ERROR (Status)) {
• DEBUG ((DEBUG_ERROR, "SetVariable BootOrderBak %r!\n", Status));
• }
• FreePool (BootOrder);
• return;

+}

+VOID +RestoreBootOrderOnReadyToBoot (

• IN EFI_EVENT Event,
• IN VOID *Context
• )

+{

• // restore BootOrder variable in normal condition.
• RestoreBootOrder ();

+}

+STATIC +VOID +SetBootOrder (

• IN UINT16 BootType
• )

+{

• EFI_STATUS Status;
• EFI_EVENT Event;
• UINT16 *Buf;
• UINT16 *NewOrder;
• UINT16 *RemainBoots;
• UINT16 *BootOrder;
• UINTN BootOrderSize;
• CHAR16 OptionName[sizeof ("Boot####")];
• UINTN Index;
• LIST_ENTRY BootOptionList;
• BDS_COMMON_OPTION *Option;
• UINTN BootCnt;
• UINTN SelectCnt;
• UINTN RemainCnt;
• InitializeListHead (&BootOptionList);
• GetEfiGlobalVariable2 (L"BootOrder", (VOID **) &BootOrder, &BootOrderSize);
• if (BootOrder == NULL) {
• return ;
• }
• Buf = (UINT16 *) AllocatePool (BootOrderSize * 2);
• if (Buf == NULL) {
• DEBUG ((DEBUG_ERROR, "Out of resources."));
• goto Exit;
• }
• NewOrder = Buf;
• BootCnt = BootOrderSize / sizeof (UINT16);
• RemainBoots = Buf + BootCnt;

OK, so the code above is really confusing. And we still have this spurious variable "Buf", that is never used.

I would propose to rewrite the above as

NewOrder = AllocatePool (BootOrderSize); RemainBoots = AllocatePool (BootOrderSize); if ((NewOrder == NULL) || (RemainBoots == NULL)) { DEBUG ((DEBUG_ERROR, "Out of resources.")); goto Exit; }

• SelectCnt = 0;
• RemainCnt = 0;
• for (Index = 0; Index < BootCnt; Index++) {
• UnicodeSPrint (OptionName, sizeof (OptionName), L"Boot%04x", BootOrder[Index]);
• Option = BdsLibVariableToOption (&BootOptionList, OptionName);
• if (Option == NULL) {

•  DEBUG ((DEBUG_ERROR, "Boot%04x is invalid option!\n", BootOrder[Index]));
  

•  continue;
  

• }
• if (GetBBSTypeByDevicePath (Option->DevicePath) == BootType) {

•  NewOrder[SelectCnt++] = BootOrder[Index];
  

• } else {

•  RemainBoots[RemainCnt++] = BootOrder[Index];
  

• }
• }
• if (SelectCnt != 0) {
• // append RemainBoots to NewOrder
• for (Index = 0; Index < RemainCnt; Index++) {

•  NewOrder[SelectCnt + Index] = RemainBoots[Index];
  

• }
• if (CompareMem (NewOrder, BootOrder, BootOrderSize) != 0) {

Everything from here: <<<

•  Status = gRT->SetVariable (
  

•                  L"BootOrderBak",
  

"BootOrderBackup" would be more clear, and more conformant to style.

•                  &gOemBootVaraibleGuid,
  

•                  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
  

If we are keeping this also to be able to restore after an aborted boot attempt, we explicitly want it during boot services only, so I do not think it should have the EFI_VARIABLE_RUNTIME_ACCESS attribute.

•                  BootOrderSize,
  

•                  BootOrder
  

•                  );
  

•  if (EFI_ERROR (Status)) {
  

•    goto Exit;
  

•  }
  

•  Status = gRT->SetVariable (
  

•                  L"BootOrder",
  

•                  &gEfiGlobalVariableGuid,
  

•                  EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
  

•                  BootOrderSize,
  

•                  NewOrder
  

•                  );
  

•  if (EFI_ERROR (Status)) {
  

•    goto Exit;
  

•  }
  

•  // Register notify function to restore BootOrder variable on ReadyToBoot Event.
  

•  Status = gBS->CreateEventEx (
  

•                  EVT_NOTIFY_SIGNAL,
  

•                  TPL_CALLBACK,
  

•                  RestoreBootOrderOnReadyToBoot,
  

•                  NULL,
  

•                  &gEfiEventReadyToBootGuid,
  

•                  &Event
  

•                  );
  

•  if (EFI_ERROR (Status)) {
  

•    DEBUG ((DEBUG_ERROR, "Create ready to boot event %r!\n", Status));
  

•  }
  

• }

...

...

To here, should go into an UpdateBootOrder() helper function.

• }

+Exit:

• FreePool (BootOrder);
• if (Buf != NULL) {
• FreePool (Buf);
• }

And here: if (NewOrder != NULL) { FreePool (NewOrder); } if (RemainBoots != NULL) { FreePool (RemainBoots); }

• return ;

+}

+STATIC +VOID +HandleBmcBootType (

• VOID
• )

+{

• EFI_STATUS Status;
• IPMI_GET_BOOT_OPTION BmcBootOpt;
• UINT16 BootType;
• Status = GetBmcBootOptionsSetting (&BmcBootOpt);
• if (EFI_ERROR (Status)) {
• return;
• }
• Print (L"Boot Type from BMC is %x\n", BmcBootOpt.BootDeviceSelector);
• switch (BmcBootOpt.BootDeviceSelector) {
• case ForcePxe:
• BootType = BBS_TYPE_EMBEDDED_NETWORK;
• break;
• case ForcePrimaryRemovableMedia:
• BootType = BBS_TYPE_USB;
• break;
• case ForceDefaultHardDisk:
• BootType = BBS_TYPE_HARDDRIVE;
• break;
• case ForceDefaultCD:
• BootType = BBS_TYPE_CDROM;
• break;
• default:
• return;
• }
• SetBootOrder (BootType);

+} // // BDS Platform Functions @@ -159,6 +578,9 @@ PlatformBdsInit ( { //Signal EndofDxe Event EfiEventGroupSignal(&gEfiEndOfDxeEventGroupGuid);

• // restore BootOrder variable if necessary.

I would prefer for this comment to be reworded as:

// restore BootOrder variable if previous BMC boot override attempt // left it in a modified state

(If I knew why it was necessary, I would not need the comment - so the existing one is not helpful.)

/ Leif

• RestoreBootOrder ();

} @@ -473,6 +895,7 @@ PlatformBdsPolicyBehavior ( Print (L"Press Enter to boot OS immediately.\n"); Print (L"Press any other key in %d seconds to stop automatical booting...\n", PcdGet16(PcdPlatformBootTimeOut)); PlatformBdsEnterFrontPage (PcdGet16(PcdPlatformBootTimeOut), TRUE);

• HandleBmcBootType ();

} /** diff --git a/Chips/Hisilicon/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf b/Chips/Hisilicon/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf index baceb57..41ceb89 100644 --- a/Chips/Hisilicon/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf +++ b/Chips/Hisilicon/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf @@ -42,6 +42,7 @@ IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec MdeModulePkg/MdeModulePkg.dec MdePkg/MdePkg.dec

• OpenPlatformPkg/Chips/Hisilicon/HisiPkg.dec

[LibraryClasses] BaseLib @@ -49,6 +50,7 @@ DebugLib DevicePathLib GenericBdsLib

• IpmiCmdLib MemoryAllocationLib PcdLib PrintLib

@@ -68,12 +70,14 @@ [Guids] gEfiEndOfDxeEventGroupGuid

• gEfiEventReadyToBootGuid gEfiFileInfoGuid gEfiFileSystemInfoGuid gEfiFileSystemVolumeLabelInfoIdGuid

[Protocols] gEfiDevicePathProtocolGuid

• gEfiDevicePathToTextProtocolGuid gEfiGraphicsOutputProtocolGuid gEfiLoadedImageProtocolGuid gEfiPciRootBridgeIoProtocolGuid

-- 1.9.1