Add a plug-in library for UiApp that creates a 'UEFI Shell' menu
option at the root level which gives access to a form that allows
the UEFI Shell to be launched.

This gives the PlatformBootManagerLib implementation of the platform
more flexibility in the way it handles boot options pointing to the
UEFI Shell, which will typically be invoked with only the boot path
connected on fast boots.

This library may be incorporated to a platform build by adding a
NULL resolution to the <LibraryClasses> section of the UiApp.inf
{} block in the platform .DSC

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf |  45 ++++
 ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h   |  44 ++++
 ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c   | 258 ++++++++++++++++++++
 ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni      |  17 ++
 ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr          |  37 +++
 5 files changed, 401 insertions(+)

diff --git a/ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf b/ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf
new file mode 100644
index 000000000000..d8b56268c08f
--- /dev/null
+++ b/ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf
@@ -0,0 +1,45 @@
+## @file
+#
+#  Boot Maintenance Manager Library to add a 'UEFI Shell' option to UiApp
+#
+#  Copyright (c) 2020, Arm Ltd. All rights reserved.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 1.27
+  BASE_NAME                      = ShellBootManagerLib
+  FILE_GUID                      = f84d949a-1ffd-447e-903d-5def3c34040b
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NULL|DXE_DRIVER UEFI_APPLICATION
+  CONSTRUCTOR                    = ShellBootManagerLibConstructor
+  DESTRUCTOR                     = ShellBootManagerLibDestructor
+
+[Sources]
+  ShellBootManagerLib.c
+  ShellBootManagerLib.h
+  ShellBmVfr.Vfr
+  ShellBmStrings.uni
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  ShellPkg/ShellPkg.dec
+
+[LibraryClasses]
+  DebugLib
+  DevicePathLib
+  DxeServicesLib
+  HiiLib
+  MemoryAllocationLib
+  UefiBootServicesTableLib
+
+[Guids]
+  gEfiIfrFrontPageGuid                          ## CONSUMES ## GUID
+  gUefiShellFileGuid                            ## CONSUMES ## GUID
+
+[Protocols]
+  gEfiHiiConfigAccessProtocolGuid               ## PRODUCES
+  gEfiDevicePathProtocolGuid                    ## PRODUCES
diff --git a/ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h b/ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h
new file mode 100644
index 000000000000..e9cdfb6a8a64
--- /dev/null
+++ b/ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h
@@ -0,0 +1,44 @@
+/** @file
+
+  Boot Maintenance Manager Library to add a 'UEFI Shell' option to UiApp
+
+  Copyright (c) 2020, Arm Ltd. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi/UefiBaseType.h>
+
+#include <Protocol/DevicePath.h>
+#include <Protocol/HiiConfigAccess.h>
+
+extern UINT8 ShellBmVfrBin[];
+
+#define FORMSET_GUID  { 0x54335e64, 0x4ebc, 0x4f7d, { 0x8a, 0x9a, 0x94, 0x10, 0xf5, 0x53, 0xae, 0x9d } }
+
+///
+/// HII specific Vendor Device Path definition.
+///
+#pragma pack(1)
+typedef struct {
+  VENDOR_DEVICE_PATH             VendorDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL       End;
+} HII_VENDOR_DEVICE_PATH;
+#pragma pack()
+
+#define SHELL_BM_CALLBACK_DATA_SIGNATURE  SIGNATURE_32 ('S', 'B', 'C', 'D')
+
+typedef struct {
+  UINTN                           Signature;
+
+  //
+  // HII relative handles
+  //
+  EFI_HII_HANDLE                  HiiHandle;
+  EFI_HANDLE                      DriverHandle;
+
+  //
+  // Produced protocols
+  //
+  EFI_HII_CONFIG_ACCESS_PROTOCOL   ConfigAccess;
+} SHELL_BM_CALLBACK_DATA;
diff --git a/ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c b/ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c
new file mode 100644
index 000000000000..195f50601dc0
--- /dev/null
+++ b/ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c
@@ -0,0 +1,258 @@
+/** @file
+
+  Boot Maintenance Manager Library to add a 'UEFI Shell' option to UiApp
+
+  Copyright (c) 2020, Arm Ltd. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiDxe.h>
+
+#include <Library/DebugLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/DxeServicesLib.h>
+#include <Library/HiiLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+
+#include "ShellBootManagerLib.h"
+
+STATIC CONST EFI_GUID             mFormsetGuid = FORMSET_GUID;
+STATIC EFI_DEVICE_PATH_PROTOCOL   *mShellFileDevicePath;
+
+STATIC HII_VENDOR_DEVICE_PATH     mShellBmHiiVendorDevicePath = {
+  {
+    {
+      HARDWARE_DEVICE_PATH,
+      HW_VENDOR_DP,
+      {
+        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
+        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
+      }
+    },
+    //
+    // File GUID: f84d949a-1ffd-447e-903d-5def3c34040b
+    //
+    { 0xf84d949a, 0x1ffd, 0x447e, { 0x90, 0x3d, 0x5d, 0xef, 0x3c, 0x34, 0x04, 0x0b } }
+  },
+  {
+    END_DEVICE_PATH_TYPE,
+    END_ENTIRE_DEVICE_PATH_SUBTYPE,
+    {
+      (UINT8) (END_DEVICE_PATH_LENGTH),
+      (UINT8) ((END_DEVICE_PATH_LENGTH) >> 8)
+    }
+  }
+};
+
+/**
+  This function allows a caller to extract the current configuration for one
+  or more named elements from the target driver.
+
+
+  @param This            Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL.
+  @param Request         A null-terminated Unicode string in <ConfigRequest> format.
+  @param Progress        On return, points to a character in the Request string.
+                         Points to the string's null terminator if request was successful.
+                         Points to the most recent '&' before the first failing name/value
+                         pair (or the beginning of the string if the failure is in the
+                         first name/value pair) if the request was not successful.
+  @param Results         A null-terminated Unicode string in <ConfigAltResp> format which
+                         has all values filled in for the names in the Request string.
+                         String to be allocated by the called function.
+
+  @retval  EFI_SUCCESS            The Results is filled with the requested values.
+  @retval  EFI_OUT_OF_RESOURCES   Not enough memory to store the results.
+  @retval  EFI_INVALID_PARAMETER  Request is illegal syntax, or unknown name.
+  @retval  EFI_NOT_FOUND          Routing data doesn't match any storage in this driver.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ShellBmExtractConfig (
+  IN  CONST EFI_HII_CONFIG_ACCESS_PROTOCOL   *This,
+  IN  CONST EFI_STRING                       Request,
+  OUT EFI_STRING                             *Progress,
+  OUT EFI_STRING                             *Results
+  )
+{
+  if (Progress == NULL || Results == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+  *Progress = Request;
+  return EFI_NOT_FOUND;
+}
+
+/**
+  This function processes the results of changes in configuration.
+
+
+  @param This            Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL.
+  @param Configuration   A null-terminated Unicode string in <ConfigResp> format.
+  @param Progress        A pointer to a string filled in with the offset of the most
+                         recent '&' before the first failing name/value pair (or the
+                         beginning of the string if the failure is in the first
+                         name/value pair) or the terminating NULL if all was successful.
+
+  @retval  EFI_SUCCESS            The Results is processed successfully.
+  @retval  EFI_INVALID_PARAMETER  Configuration is NULL.
+  @retval  EFI_NOT_FOUND          Routing data doesn't match any storage in this driver.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ShellBmRouteConfig (
+  IN  CONST EFI_HII_CONFIG_ACCESS_PROTOCOL   *This,
+  IN  CONST EFI_STRING                       Configuration,
+  OUT EFI_STRING                             *Progress
+  )
+{
+  if (Configuration == NULL || Progress == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *Progress = Configuration;
+
+  return EFI_NOT_FOUND;
+}
+
+/**
+  This function processes the results of changes in configuration.
+
+
+  @param This            Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL.
+  @param Action          Specifies the type of action taken by the browser.
+  @param QuestionId      A unique value which is sent to the original exporting driver
+                         so that it can identify the type of data to expect.
+  @param Type            The type of value for the question.
+  @param Value           A pointer to the data being sent to the original exporting driver.
+  @param ActionRequest   On return, points to the action requested by the callback function.
+
+  @retval  EFI_SUCCESS           The callback successfully handled the action.
+  @retval  EFI_OUT_OF_RESOURCES  Not enough storage is available to hold the variable and its data.
+  @retval  EFI_DEVICE_ERROR      The variable could not be saved.
+  @retval  EFI_UNSUPPORTED       The specified Action is not supported by the callback.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+ShellBmHiiCallback (
+  IN  CONST EFI_HII_CONFIG_ACCESS_PROTOCOL   *This,
+  IN  EFI_BROWSER_ACTION                     Action,
+  IN  EFI_QUESTION_ID                        QuestionId,
+  IN  UINT8                                  Type,
+  IN  EFI_IFR_TYPE_VALUE                     *Value,
+  OUT EFI_BROWSER_ACTION_REQUEST             *ActionRequest
+  )
+{
+  EFI_HANDLE    ImageHandle;
+  EFI_STATUS    Status;
+
+  if (Action != EFI_BROWSER_ACTION_CHANGED) {
+    return EFI_SUCCESS;
+  }
+
+  Status = gBS->LoadImage (TRUE, gImageHandle, mShellFileDevicePath, NULL, 0,
+                  &ImageHandle);
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Clear  the  screen  before.
+  //
+  gST->ConOut->SetAttribute (gST->ConOut, EFI_TEXT_ATTR (EFI_LIGHTGRAY, EFI_BLACK));
+  gST->ConOut->ClearScreen (gST->ConOut);
+
+  Status = gBS->StartImage (ImageHandle, NULL, NULL);
+  ASSERT_EFI_ERROR (Status);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN, "%a: UEFI Shell returned with status %r\n",
+      __FUNCTION__, Status));
+  }
+  return EFI_SUCCESS;
+}
+
+STATIC SHELL_BM_CALLBACK_DATA     mShellBmPrivate = {
+  SHELL_BM_CALLBACK_DATA_SIGNATURE,
+  NULL,
+  NULL,
+  {
+    ShellBmExtractConfig,
+    ShellBmRouteConfig,
+    ShellBmHiiCallback
+  }
+};
+
+EFI_STATUS
+EFIAPI
+ShellBootManagerLibConstructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS      Status;
+
+  Status = GetFileDevicePathFromAnyFv (&gUefiShellFileGuid, EFI_SECTION_PE32,
+             0, &mShellFileDevicePath);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_WARN, "%a: failed to locate UEFI shell by GUID %g - %r\n",
+      __FUNCTION__, &gUefiShellFileGuid, Status));
+    return EFI_SUCCESS;
+  }
+
+  //
+  // Install Device Path Protocol and Config Access protocol to driver handle
+  //
+  mShellBmPrivate.DriverHandle = NULL;
+  Status = gBS->InstallMultipleProtocolInterfaces (
+                  &mShellBmPrivate.DriverHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  &mShellBmHiiVendorDevicePath,
+                  &gEfiHiiConfigAccessProtocolGuid,
+                  &mShellBmPrivate.ConfigAccess,
+                  NULL
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Publish our HII data
+  //
+  mShellBmPrivate.HiiHandle = HiiAddPackages (
+                                &mFormsetGuid,
+                                mShellBmPrivate.DriverHandle,
+                                ShellBmVfrBin,
+                                ShellBootManagerLibStrings,
+                                NULL
+                                );
+  ASSERT (mShellBmPrivate.HiiHandle != NULL);
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+ShellBootManagerLibDestructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS      Status;
+
+  Status = gBS->UninstallMultipleProtocolInterfaces (
+                  mShellBmPrivate.DriverHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  &mShellBmHiiVendorDevicePath,
+                  &gEfiHiiConfigAccessProtocolGuid,
+                  &mShellBmPrivate.ConfigAccess,
+                  NULL
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  HiiRemovePackages (mShellBmPrivate.HiiHandle);
+
+  FreePool (mShellFileDevicePath);
+  return EFI_SUCCESS;
+}
diff --git a/ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni b/ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni
new file mode 100644
index 000000000000..6e1962b6d0ec
--- /dev/null
+++ b/ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni
@@ -0,0 +1,17 @@
+///** @file
+//
+// Copyright (c) 2020, Arm Ltd. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// --*/
+
+/=#
+#langdef   en-US "English"
+
+#string STR_SHELL_BM_BANNER            #language en-US  "UEFI Shell"
+#string STR_SHELL_BM_HELP              #language en-US  "This selection will take you to the UEFI Shell"
+#string STR_SHELL_BM_BANNER_FORM_TITLE #language en-US  "UEFI Shell Menu"
+#string STR_SHELL_BM_ENTER_SHELL       #language en-US  "Enter the UEFI Shell"
+#string STR_SHELL_BM_ENTER_SHELL_HELP  #language en-US  "Select this option to enter the UEFI Shell"
+#string STR_LAST_STRING                #language en-US  ""
diff --git a/ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr b/ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr
new file mode 100644
index 000000000000..bdff98c7c07a
--- /dev/null
+++ b/ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr
@@ -0,0 +1,37 @@
+///** @file
+//
+//  UEFI Shell Boot Manager formset.
+//
+//  Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.<BR>
+//  Copyright (c) 2020, Arm Ltd. All rights reserved.<BR>
+//
+//  SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+//**/
+
+#define FORMSET_GUID  { 0x54335e64, 0x4ebc, 0x4f7d, 0x8a, 0x9a, 0x94, 0x10, 0xf5, 0x53, 0xae, 0x9d }
+
+#define BOOT_MANAGER_FORM_ID     0x1000
+
+formset
+  guid      = FORMSET_GUID,
+  title     = STRING_TOKEN(STR_SHELL_BM_BANNER),
+  help      = STRING_TOKEN(STR_SHELL_BM_HELP),
+  classguid = gEfiIfrFrontPageGuid,
+
+  form formid = BOOT_MANAGER_FORM_ID,
+       title  = STRING_TOKEN(STR_SHELL_BM_BANNER);
+
+    subtitle text = STRING_TOKEN(STR_LAST_STRING);
+    subtitle text = STRING_TOKEN(STR_SHELL_BM_BANNER_FORM_TITLE);
+    subtitle text = STRING_TOKEN(STR_LAST_STRING);
+
+    text
+        help  = STRING_TOKEN(STR_SHELL_BM_ENTER_SHELL_HELP),
+        text  = STRING_TOKEN(STR_SHELL_BM_ENTER_SHELL),
+        flags = INTERACTIVE,
+        key   = 0x1;
+
+  endform;
+
+endformset;
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60291): https://edk2.groups.io/g/devel/message/60291
Mute This Topic: https://groups.io/mt/74481040/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 05/26/20 18:13, Ard Biesheuvel wrote:
> Add a plug-in library for UiApp that creates a 'UEFI Shell' menu
> option at the root level which gives access to a form that allows
> the UEFI Shell to be launched.
>
> This gives the PlatformBootManagerLib implementation of the platform
> more flexibility in the way it handles boot options pointing to the
> UEFI Shell, which will typically be invoked with only the boot path
> connected on fast boots.
>
> This library may be incorporated to a platform build by adding a
> NULL resolution to the <LibraryClasses> section of the UiApp.inf
> {} block in the platform .DSC
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> ---
>  ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf |  45 ++++
>  ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h   |  44 ++++
>  ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c   | 258 ++++++++++++++++++++
>  ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni      |  17 ++
>  ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr          |  37 +++
>  5 files changed, 401 insertions(+)

I've had to go back to the blurb and re-read this part, to understand
the goal of this patch:

> - finally, add a plugin library for UiApp to expose the UEFI Shell via an
>   ordinary main menu option (this works around the fact that patch #3 will
>   result in the UEFI Shell disappearing from the Boot Manager list).
>   Entering the shell this way will resemble the old situation, given that
>   UiApp connects all devices and refreshes all boot options etc at launch.

If I understand correctly:

- patch #3 does two things: it clears LOAD_OPTION_ACTIVE (preventing the
  boot manager from auto-booting the shell), and sets LOAD_OPTION_HIDDEN
  (hiding the boot option from UiApp),

- patch #5 undoes LOAD_OPTION_HIDDEN, in effect -- it makes sure that we
  still see the shell option "somewhere" in UiApp (not among the boot
  options, but at the root level)

Can we:

- drop patch#5, and

- pass zero (0) as "Attributes" to PlatformRegisterFvBootOption() in
  patch#3, rather than LOAD_OPTION_HIDDEN?

Because, per spec, Attributes=0 should prevent the auto-booting of the
shell *without* hiding the shell boot option from the menu.

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60361): https://edk2.groups.io/g/devel/message/60361
Mute This Topic: https://groups.io/mt/74481040/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 5/27/20 5:57 PM, Laszlo Ersek wrote:
> On 05/26/20 18:13, Ard Biesheuvel wrote:
>> Add a plug-in library for UiApp that creates a 'UEFI Shell' menu
>> option at the root level which gives access to a form that allows
>> the UEFI Shell to be launched.
>>
>> This gives the PlatformBootManagerLib implementation of the platform
>> more flexibility in the way it handles boot options pointing to the
>> UEFI Shell, which will typically be invoked with only the boot path
>> connected on fast boots.
>>
>> This library may be incorporated to a platform build by adding a
>> NULL resolution to the <LibraryClasses> section of the UiApp.inf
>> {} block in the platform .DSC
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> ---
>>   ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf |  45 ++++
>>   ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h   |  44 ++++
>>   ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c   | 258 ++++++++++++++++++++
>>   ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni      |  17 ++
>>   ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr          |  37 +++
>>   5 files changed, 401 insertions(+)
> 
> I've had to go back to the blurb and re-read this part, to understand
> the goal of this patch:
> 
>> - finally, add a plugin library for UiApp to expose the UEFI Shell via an
>>    ordinary main menu option (this works around the fact that patch #3 will
>>    result in the UEFI Shell disappearing from the Boot Manager list).
>>    Entering the shell this way will resemble the old situation, given that
>>    UiApp connects all devices and refreshes all boot options etc at launch.
> 
> If I understand correctly:
> 
> - patch #3 does two things: it clears LOAD_OPTION_ACTIVE (preventing the
>    boot manager from auto-booting the shell), and sets LOAD_OPTION_HIDDEN
>    (hiding the boot option from UiApp),
> 
> - patch #5 undoes LOAD_OPTION_HIDDEN, in effect -- it makes sure that we
>    still see the shell option "somewhere" in UiApp (not among the boot
>    options, but at the root level)
> 
> Can we:
> 
> - drop patch#5, and
> 
> - pass zero (0) as "Attributes" to PlatformRegisterFvBootOption() in
>    patch#3, rather than LOAD_OPTION_HIDDEN?
> 
> Because, per spec, Attributes=0 should prevent the auto-booting of the
> shell *without* hiding the shell boot option from the menu.
> 

I feel slightly silly having gone through all the trouble of writing 
this patch. I tried playing with the ACTIVE and HIDDEN options, and 
couldn't get this to work. If I understand these quotes correctly, this 
is an error, and instead of working around this, we should apply the 
following patch to correct it:

--- a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
+++ b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
@@ -537,7 +537,7 @@ UpdateBootManager (
      //
      // Don't display the hidden/inactive boot option
      //
-    if (((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) || 
((BootOption[Index].Attributes & LOAD_OPTION_ACTIVE) == 0)) {
+    if (((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0)) {
        continue;
      }


With this change applied, adding the shell option without the 'active' 
or 'hidden' flags works as expected: it appears in the boot manager 
menu, but is not booted automatically.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60364): https://edk2.groups.io/g/devel/message/60364
Mute This Topic: https://groups.io/mt/74481040/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 05/27/20 19:22, Ard Biesheuvel wrote:
> On 5/27/20 5:57 PM, Laszlo Ersek wrote:
>> On 05/26/20 18:13, Ard Biesheuvel wrote:
>>> Add a plug-in library for UiApp that creates a 'UEFI Shell' menu
>>> option at the root level which gives access to a form that allows
>>> the UEFI Shell to be launched.
>>>
>>> This gives the PlatformBootManagerLib implementation of the platform
>>> more flexibility in the way it handles boot options pointing to the
>>> UEFI Shell, which will typically be invoked with only the boot path
>>> connected on fast boots.
>>>
>>> This library may be incorporated to a platform build by adding a
>>> NULL resolution to the <LibraryClasses> section of the UiApp.inf
>>> {} block in the platform .DSC
>>>
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>> ---
>>>   ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.inf |  45
>>> ++++
>>>   ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.h   |  44
>>> ++++
>>>   ShellPkg/Library/ShellBootManagerLib/ShellBootManagerLib.c   | 258
>>> ++++++++++++++++++++
>>>   ShellPkg/Library/ShellBootManagerLib/ShellBmStrings.uni      |  17 ++
>>>   ShellPkg/Library/ShellBootManagerLib/ShellBmVfr.Vfr          |  37 +++
>>>   5 files changed, 401 insertions(+)
>>
>> I've had to go back to the blurb and re-read this part, to understand
>> the goal of this patch:
>>
>>> - finally, add a plugin library for UiApp to expose the UEFI Shell
>>> via an
>>>    ordinary main menu option (this works around the fact that patch
>>> #3 will
>>>    result in the UEFI Shell disappearing from the Boot Manager list).
>>>    Entering the shell this way will resemble the old situation, given
>>> that
>>>    UiApp connects all devices and refreshes all boot options etc at
>>> launch.
>>
>> If I understand correctly:
>>
>> - patch #3 does two things: it clears LOAD_OPTION_ACTIVE (preventing the
>>    boot manager from auto-booting the shell), and sets LOAD_OPTION_HIDDEN
>>    (hiding the boot option from UiApp),
>>
>> - patch #5 undoes LOAD_OPTION_HIDDEN, in effect -- it makes sure that we
>>    still see the shell option "somewhere" in UiApp (not among the boot
>>    options, but at the root level)
>>
>> Can we:
>>
>> - drop patch#5, and
>>
>> - pass zero (0) as "Attributes" to PlatformRegisterFvBootOption() in
>>    patch#3, rather than LOAD_OPTION_HIDDEN?
>>
>> Because, per spec, Attributes=0 should prevent the auto-booting of the
>> shell *without* hiding the shell boot option from the menu.
>>
> 
> I feel slightly silly having gone through all the trouble of writing
> this patch. I tried playing with the ACTIVE and HIDDEN options, and
> couldn't get this to work. If I understand these quotes correctly, this
> is an error, and instead of working around this, we should apply the
> following patch to correct it:
> 
> --- a/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> +++ b/MdeModulePkg/Library/BootManagerUiLib/BootManager.c
> @@ -537,7 +537,7 @@ UpdateBootManager (
>      //
>      // Don't display the hidden/inactive boot option
>      //
> -    if (((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0) ||
> ((BootOption[Index].Attributes & LOAD_OPTION_ACTIVE) == 0)) {
> +    if (((BootOption[Index].Attributes & LOAD_OPTION_HIDDEN) != 0)) {
>        continue;
>      }
> 
> 
> With this change applied, adding the shell option without the 'active'
> or 'hidden' flags works as expected: it appears in the boot manager
> menu, but is not booted automatically.

I enthusiastically agree that we should apply your above
BootManagerUiLib patch. I don't see why (per spec) the UI should hide a
boot option just because it is inactive.

Thanks!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60409): https://edk2.groups.io/g/devel/message/60409
Mute This Topic: https://groups.io/mt/74481040/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-