[PATCH v4 0/2] add platform boot manager protocol

[PATCH v4 0/2] add platform boot manager protocol

[Haojian Zhuang]

Changelog:
v4:
  * Add BootCount parameter in the interface.
  * Clean the logic on boot options according to Laszlo's comment.
v3:
  * Update the name of interface.
  * Move the initialization into platform driver.
  * Fix comment style.
  * Fix minor issues with comments.
v2:
  * Avoid to use hardcoding value. Create boot options by functions.

Haojian Zhuang (2):
  EmbeddedPkg: add platform boot manager protocol
  ArmPkg/PlatformBootManagerLib: load platform boot options

 ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 103 +++++++++++++++++++++
 .../PlatformBootManagerLib.inf                     |   2 +
 EmbeddedPkg/EmbeddedPkg.dec                        |   1 +
 EmbeddedPkg/Include/Protocol/PlatformBootManager.h |  86 +++++++++++++++++
 4 files changed, 192 insertions(+)
 create mode 100644 EmbeddedPkg/Include/Protocol/PlatformBootManager.h

-- 
2.7.4

[PATCH v4 1/2] EmbeddedPkg: add platform boot manager protocol

[Haojian Zhuang]

Create the PlatformBootManagerProtocol that is used to add
predefined boot options in platform driver. This interface
will be used in ArmPkg/PlatformBootManagerLib.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dec                        |  1 +
 EmbeddedPkg/Include/Protocol/PlatformBootManager.h | 86 ++++++++++++++++++++++
 2 files changed, 87 insertions(+)
 create mode 100644 EmbeddedPkg/Include/Protocol/PlatformBootManager.h

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index eb135340b173..9cd50738363b 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -81,6 +81,7 @@ [Protocols.common]
   gPlatformGpioProtocolGuid = { 0x52ce9845, 0x5af4, 0x43e2, {0xba, 0xfd, 0x23, 0x08, 0x12, 0x54, 0x7a, 0xc2 }}
   gAndroidBootImgProtocolGuid = { 0x9859bb19, 0x407c, 0x4f8b, {0xbc, 0xe1, 0xf8, 0xda, 0x65, 0x65, 0xf4, 0xa5 }}
   gPlatformVirtualKeyboardProtocolGuid = { 0x0e3606d2, 0x1dc3, 0x4e6f, { 0xbe, 0x65, 0x39, 0x49, 0x82, 0xa2, 0x65, 0x47 }}
+  gPlatformBootManagerProtocolGuid = { 0x7197c8a7, 0x6559, 0x4c93, { 0x93, 0xd5, 0x8a, 0x84, 0xf9, 0x88, 0x79, 0x8b }}
 
 [Ppis]
   gEdkiiEmbeddedGpioPpiGuid = { 0x21c3b115, 0x4e0b, 0x470c, { 0x85, 0xc7, 0xe1, 0x05, 0xa5, 0x75, 0xc9, 0x7b }}
diff --git a/EmbeddedPkg/Include/Protocol/PlatformBootManager.h b/EmbeddedPkg/Include/Protocol/PlatformBootManager.h
new file mode 100644
index 000000000000..9fa27985b469
--- /dev/null
+++ b/EmbeddedPkg/Include/Protocol/PlatformBootManager.h
@@ -0,0 +1,86 @@
+/** @file
+
+  Copyright (c) 2018, Linaro. 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.
+
+**/
+
+#ifndef __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
+#define __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
+
+//
+// Protocol interface structure
+//
+typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL    PLATFORM_BOOT_MANAGER_PROTOCOL;
+
+//
+// Function Prototypes
+//
+
+/*
+  Get predefined boot options for platform.
+
+  @param[out] Count            The number of elements in each of
+                               BootOptions and BootKeys. On successful
+                               return, Count is at least one.
+
+  @param[out] BootOptions      An array of platform boot options.
+                               BootOptions is pool-allocated by
+                               GET_PLATFORM_BOOT_OPTIONS_AND_KEYS, and
+                               GET_PLATFORM_BOOT_OPTIONS_AND_KEYS populates
+                               every element in BootOptions with
+                               EfiBootManagerInitializeLoadOption().
+                               BootOptions shall not contain duplicate
+                               entries. The caller is responsible for
+                               releasing BootOptions after use with
+                               EfiBootManagerFreeLoadOptions().
+
+  @param[out] BootKeys         A pool-allocated array of platform boot
+                               hotkeys. For every 0 <= Index < Count, if
+                               BootOptions[Index] is not to be associated
+                               with a hotkey, then BootKeys[Index] is
+                               zero-filled. Otherwise, BootKeys[Index]
+                               specifies the boot hotkey for
+                               BootOptions[Index]. BootKeys shall not
+                               contain duplicate entries (other than
+                               zero-filled entries). The caller is
+                               responsible for releasing BootKeys with
+                               FreePool() after use.
+
+  @retval EFI_SUCCESS          Count, BootOptions and BootKeys have
+                               been set.
+
+  @retval EFI_OUT_OF_RESOURCES Memory allocation failed.
+
+  @retval EFI_UNSUPPORTED      The platform doesn't provide boot options
+                               as a feature.
+
+  @retval EFI_NOT_FOUND        The platform could provide boot options
+                               as a feature, but none have been
+                               configured.
+
+  @return                      Error codes propagated from underlying
+                               functions.
+*/
+typedef
+EFI_STATUS
+(EFIAPI *GET_PLATFORM_BOOT_OPTIONS_AND_KEYS) (
+  OUT UINTN                              *Count,
+  OUT EFI_BOOT_MANAGER_LOAD_OPTION       **BootOptions,
+  OUT EFI_INPUT_KEY                      **BootKeys
+  );
+
+struct _PLATFORM_BOOT_MANAGER_PROTOCOL {
+  GET_PLATFORM_BOOT_OPTIONS_AND_KEYS     GetPlatformBootOptionsAndKeys;
+};
+
+extern EFI_GUID gPlatformBootManagerProtocolGuid;
+
+#endif /* __PLATFORM_BOOT_MANAGER_PROTOCOL_H__ */
-- 
2.7.4

[PATCH v4 2/2] ArmPkg/PlatformBootManagerLib: load platform boot options

[Haojian Zhuang]

Make platform driver to create predefined boot options and related
hot keys.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
---
 ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 103 +++++++++++++++++++++
 .../PlatformBootManagerLib.inf                     |   2 +
 2 files changed, 105 insertions(+)

diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 61ab61ccc780..3456a71fbb9c 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -30,6 +30,7 @@
 #include <Protocol/LoadedImage.h>
 #include <Protocol/PciIo.h>
 #include <Protocol/PciRootBridgeIo.h>
+#include <Protocol/PlatformBootManager.h>
 #include <Guid/EventGroup.h>
 #include <Guid/TtyTerm.h>
 
@@ -392,6 +393,106 @@ PlatformRegisterFvBootOption (
 
 STATIC
 VOID
+GetPlatformOptions (
+  VOID
+  )
+{
+  EFI_STATUS                      Status;
+  EFI_BOOT_MANAGER_LOAD_OPTION    *CurrentBootOptions;
+  EFI_BOOT_MANAGER_LOAD_OPTION    *BootOptions;
+  EFI_INPUT_KEY                   *BootKeys;
+  PLATFORM_BOOT_MANAGER_PROTOCOL  *PlatformBootManager;
+  UINTN                           CurrentBootOptionCount;
+  UINTN                           Index;
+  UINTN                           BootCount;
+
+  Status = gBS->LocateProtocol (&gPlatformBootManagerProtocolGuid, NULL,
+                  (VOID **)&PlatformBootManager);
+  if (EFI_ERROR (Status)) {
+    return;
+  }
+  Status = PlatformBootManager->GetPlatformBootOptionsAndKeys (
+                                  &BootCount,
+                                  &BootOptions,
+                                  &BootKeys
+                                  );
+  if (EFI_ERROR (Status)) {
+    return;
+  }
+  //
+  // Fetch the existent boot options. If there are none, CurrentBootCount
+  // will be zeroed.
+  //
+  CurrentBootOptions = EfiBootManagerGetLoadOptions (
+                         &CurrentBootOptionCount,
+                         LoadOptionTypeBoot
+                         );
+  //
+  // Process the platform boot options.
+  //
+  for (Index = 0; Index < BootCount; Index++) {
+    INTN    Match;
+    UINTN   BootOptionNumber;
+
+    //
+    // If there are any preexistent boot options, and the subject platform boot
+    // option is already among them, then don't try to add it. Just get its
+    // assigned boot option number so we can associate a hotkey with it. Note
+    // that EfiBootManagerFindLoadOption() deals fine with (CurrentBootOptions
+    // == NULL) if (CurrentBootCount == 0).
+    //
+    Match = EfiBootManagerFindLoadOption (
+              &BootOptions[Index],
+              CurrentBootOptions,
+              CurrentBootOptionCount
+              );
+    if (Match >= 0) {
+      BootOptionNumber = CurrentBootOptions[Match].OptionNumber;
+    } else {
+      //
+      // Add the platform boot options as a new one, at the end of the boot
+      // order. Note that if the platform provided this boot option with an
+      // unassigned option number, then the below function call will assign a
+      // number.
+      //
+      Status = EfiBootManagerAddLoadOptionVariable (
+                 &BootOptions[Index],
+                 MAX_UINTN
+                 );
+      if (EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_ERROR, "%a: failed to register \"%s\": %r\n",
+          __FUNCTION__, BootOptions[Index].Description, Status));
+        continue;
+      }
+      BootOptionNumber = BootOptions[Index].OptionNumber;
+    }
+
+    //
+    // Register a hotkey with the boot option, if requested.
+    //
+    if (BootKeys[Index].UnicodeChar == L'\0') {
+      continue;
+    }
+
+    Status = EfiBootManagerAddKeyOptionVariable (
+               NULL,
+               BootOptionNumber,
+               0,
+               BootKeys[Index],
+               NULL
+               );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR, "%a: failed to register hotkey for \"%s\": %r\n",
+        __FUNCTION__, BootOptions[Index].Description, Status));
+    }
+  }
+  EfiBootManagerFreeLoadOptions (CurrentBootOptions, CurrentBootOptionCount);
+  EfiBootManagerFreeLoadOptions (BootOptions, BootCount);
+  FreePool (BootKeys);
+}
+
+STATIC
+VOID
 PlatformRegisterOptionsAndKeys (
   VOID
   )
@@ -402,6 +503,8 @@ PlatformRegisterOptionsAndKeys (
   EFI_INPUT_KEY                Esc;
   EFI_BOOT_MANAGER_LOAD_OPTION BootOption;
 
+  GetPlatformOptions ();
+
   //
   // Register ENTER as CONTINUE key
   //
diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 71f1d04a87ee..e8cbb10dabdd 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -35,6 +35,7 @@ [Sources]
   PlatformBm.c
 
 [Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
   ShellPkg/ShellPkg.dec
@@ -84,3 +85,4 @@ [Protocols]
   gEfiPciRootBridgeIoProtocolGuid
   gEfiSimpleFileSystemProtocolGuid
   gEsrtManagementProtocolGuid
+  gPlatformBootManagerProtocolGuid
-- 
2.7.4

Re: [PATCH v4 0/2] add platform boot manager protocol

[Laszlo Ersek]

On 04/23/18 08:21, Haojian Zhuang wrote:
> Changelog:
> v4:
>   * Add BootCount parameter in the interface.
>   * Clean the logic on boot options according to Laszlo's comment.
> v3:
>   * Update the name of interface.
>   * Move the initialization into platform driver.
>   * Fix comment style.
>   * Fix minor issues with comments.
> v2:
>   * Avoid to use hardcoding value. Create boot options by functions.
> 
> Haojian Zhuang (2):
>   EmbeddedPkg: add platform boot manager protocol
>   ArmPkg/PlatformBootManagerLib: load platform boot options
> 
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 103 +++++++++++++++++++++
>  .../PlatformBootManagerLib.inf                     |   2 +
>  EmbeddedPkg/EmbeddedPkg.dec                        |   1 +
>  EmbeddedPkg/Include/Protocol/PlatformBootManager.h |  86 +++++++++++++++++
>  4 files changed, 192 insertions(+)
>  create mode 100644 EmbeddedPkg/Include/Protocol/PlatformBootManager.h
> 

(CC Leif, Ard)

For the series:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

Re: [PATCH v4 0/2] add platform boot manager protocol

[Leif Lindholm]

On Mon, Apr 23, 2018 at 10:15:41PM +0200, Laszlo Ersek wrote:
> On 04/23/18 08:21, Haojian Zhuang wrote:
> > Changelog:
> > v4:
> >   * Add BootCount parameter in the interface.
> >   * Clean the logic on boot options according to Laszlo's comment.
> > v3:
> >   * Update the name of interface.
> >   * Move the initialization into platform driver.
> >   * Fix comment style.
> >   * Fix minor issues with comments.
> > v2:
> >   * Avoid to use hardcoding value. Create boot options by functions.
> > 
> > Haojian Zhuang (2):
> >   EmbeddedPkg: add platform boot manager protocol
> >   ArmPkg/PlatformBootManagerLib: load platform boot options
> > 
> >  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 103 +++++++++++++++++++++
> >  .../PlatformBootManagerLib.inf                     |   2 +
> >  EmbeddedPkg/EmbeddedPkg.dec                        |   1 +
> >  EmbeddedPkg/Include/Protocol/PlatformBootManager.h |  86 +++++++++++++++++
> >  4 files changed, 192 insertions(+)
> >  create mode 100644 EmbeddedPkg/Include/Protocol/PlatformBootManager.h
> > 
> 
> (CC Leif, Ard)
> 
> For the series:
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks for the review Lazslo.

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Series pushed as 76022b02e8..1b6e7633ca.

I will add that the bits that go into EmbeddedPkg here do so for no
other reason than "we don't have a better place for them, and they're
certainly not ARM-specific". The case for a generic UtilityPkg is growing.

/
    Leif