[edk2-devel] [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol

Ashish Singhal posted 1 patch 4 years, 3 months ago
Failed in applying to current master (apply log)
.../Include/Protocol/PlatformBootOptions.h         | 116 +++++++++++++++++++++
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   |  52 +++++++--
.../Library/UefiBootManagerLib/InternalBm.h        |   2 +
.../UefiBootManagerLib/UefiBootManagerLib.inf      |   2 +
MdeModulePkg/MdeModulePkg.dec                      |   4 +
5 files changed, 170 insertions(+), 6 deletions(-)
create mode 100644 MdeModulePkg/Include/Protocol/PlatformBootOptions.h
[edk2-devel] [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol
Posted by Ashish Singhal 4 years, 3 months ago
Add platform boot options protocol which would have platform specific
overrides to the auto enumerated boot options during the call to
EfiBootManagerRefreshAllBootOption function in UefiBootManagerLib.

Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
---
 .../Include/Protocol/PlatformBootOptions.h         | 116 +++++++++++++++++++++
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   |  52 +++++++--
 .../Library/UefiBootManagerLib/InternalBm.h        |   2 +
 .../UefiBootManagerLib/UefiBootManagerLib.inf      |   2 +
 MdeModulePkg/MdeModulePkg.dec                      |   4 +
 5 files changed, 170 insertions(+), 6 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/PlatformBootOptions.h

diff --git a/MdeModulePkg/Include/Protocol/PlatformBootOptions.h b/MdeModulePkg/Include/Protocol/PlatformBootOptions.h
new file mode 100644
index 0000000..3e08155
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/PlatformBootOptions.h
@@ -0,0 +1,116 @@
+/** @file
+
+  Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __PLATFORM_BOOT_OPTIONS_PROTOCOL_H__
+#define __PLATFORM_BOOT_OPTIONS_PROTOCOL_H__
+
+#include <Library/UefiBootManagerLib.h>
+
+//
+// Platform Boot Options Protocol GUID value
+//
+#define EDKII_PLATFORM_BOOT_OPTIONS_PROTOCOL_GUID \
+    { \
+      0xaa17add4, 0x756c, 0x460d, { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e, 0x59 } \
+    }
+
+//
+// Protocol interface structure
+//
+typedef struct _PLATFORM_BOOT_OPTIONS_PROTOCOL PLATFORM_BOOT_OPTIONS_PROTOCOL;
+
+//
+// Revision The revision to which the protocol interface adheres.
+//          All future revisions must be backwards compatible.
+//          If a future version is not back wards compatible it is not the same GUID.
+//
+#define EDKII_PLATFORM_BOOT_OPTIONS_PROTOCOL_REVISION 0x00000001
+
+//
+// Function Prototypes
+//
+
+/*
+  This function allows overrides to auto enumerated boot options for platform.
+
+  @param[in const] BootOptionsCount        The number of elements in BootOptions.
+
+  @param[in const] BootOptions             An array of auto enumerated platform boot options.
+                                           This array will be freed by caller upon successful
+                                           exit of this function and output array would be used.
+
+  @param[out]      UpdatedBootOptionsCount The number of elements in UpdatedBootOptions.
+
+  @param[out]      UpdatedBootOptions      An array of boot options that have been customized
+                                           for the platform on top of input boot options. This
+                                           array would be allocated by OVERRIDE_PLATFORM_BOOT_OPTIONS
+                                           and would be freed by caller after consuming it.
+
+
+  @retval EFI_SUCCESS                      Platform overrides to input BootOptions and
+                                           BootCount have been done.
+
+  @retval EFI_OUT_OF_RESOURCES             Memory allocation failed.
+
+  @retval EFI_INVALID_PARAMETER            Input is not correct.
+
+  @retval EFI_UNSUPPORTED                  Platform specific overrides are not supported.
+*/
+typedef
+EFI_STATUS
+(EFIAPI *OVERRIDE_PLATFORM_BOOT_OPTIONS) (
+  IN  CONST UINTN                        BootOptionsCount,
+  IN  CONST EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions,
+  OUT       UINTN                        *UpdatedBootOptionsCount,
+  OUT       EFI_BOOT_MANAGER_LOAD_OPTION **UpdatedBootOptions
+  );
+
+/*
+  This function allows to remove invalid platform specific boot options from NV.
+
+  @param[in const] NvBootOptionsCount        The number of elements in NvBootOptions.
+
+  @param[in const] NvBootOptions             An array of current boot options in NV store.
+                                             This array will be freed by caller upon successful
+                                             exit of this function and output array would be used.
+
+  @param[out]      UpdatedBootOptionsCount   The number of elements in UpdatedBootOptions.
+
+  @param[out]      UpdatedBootOptions        An array of NV boot options that have been cleaned up
+                                             for the platform on top of input NV boot options. This
+                                             array would be allocated by REMOVE_INVALID_PLATFORM_NV_BOOT_OPTIONS
+                                             and would be freed by caller after consuming it.
+
+
+  @retval EFI_SUCCESS                        Platform cleanup to input NvBootOptions and
+                                             NvBootCount have been done.
+
+  @retval EFI_OUT_OF_RESOURCES               Memory allocation failed.
+
+  @retval EFI_INVALID_PARAMETER              Input is not correct.
+
+  @retval EFI_UNSUPPORTED                    Platform specific overrides are not supported.
+*/
+typedef
+EFI_STATUS
+(EFIAPI *REMOVE_INVALID_PLATFORM_NV_BOOT_OPTIONS) (
+  IN  CONST UINTN                        NvBootOptionsCount,
+  IN  CONST EFI_BOOT_MANAGER_LOAD_OPTION *NvBootOptions,
+  OUT       UINTN                        *UpdatedBootOptionsCount,
+  OUT       EFI_BOOT_MANAGER_LOAD_OPTION **UpdatedBootOptions
+  );
+
+struct _PLATFORM_BOOT_OPTIONS_PROTOCOL {
+  UINT64                                  Revision;
+  OVERRIDE_PLATFORM_BOOT_OPTIONS          OverridePlatformBootOptions;
+  REMOVE_INVALID_PLATFORM_NV_BOOT_OPTIONS RemoveInvalidPlatformNvBootOptions;
+};
+
+extern EFI_GUID gEdkiiPlatformBootOptionsProtocolGuid;
+
+#endif /* __PLATFORM_BOOT_OPTIONS_PROTOCOL_H__ */
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 760d764..41e09d5 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1,6 +1,7 @@
 /** @file
   Library functions which relates with booting.
 
+Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
 Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -2258,12 +2259,15 @@ EfiBootManagerRefreshAllBootOption (
   VOID
   )
 {
-  EFI_STATUS                    Status;
-  EFI_BOOT_MANAGER_LOAD_OPTION  *NvBootOptions;
-  UINTN                         NvBootOptionCount;
-  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
-  UINTN                         BootOptionCount;
-  UINTN                         Index;
+  EFI_STATUS                     Status;
+  EFI_BOOT_MANAGER_LOAD_OPTION   *NvBootOptions;
+  UINTN                          NvBootOptionCount;
+  EFI_BOOT_MANAGER_LOAD_OPTION   *BootOptions;
+  UINTN                          BootOptionCount;
+  EFI_BOOT_MANAGER_LOAD_OPTION   *UpdatedBootOptions;
+  UINTN                          UpdatedBootOptionCount;
+  UINTN                          Index;
+  PLATFORM_BOOT_OPTIONS_PROTOCOL *PlatformBootOptions;
 
   //
   // Optionally refresh the legacy boot option
@@ -2284,6 +2288,42 @@ EfiBootManagerRefreshAllBootOption (
   }
 
   //
+  // Locate Platform Boot Options Protocol
+  //
+  PlatformBootOptions = NULL;
+  Status = gBS->LocateProtocol (&gEdkiiPlatformBootOptionsProtocolGuid,
+                                NULL,
+                                (VOID **)&PlatformBootOptions);
+  if (!EFI_ERROR (Status)) {
+    //
+    // If found, call platform specific overrides to auto enumerated
+    // boot options.
+    //
+    Status = PlatformBootOptions->OverridePlatformBootOptions ((CONST UINTN)BootOptionCount,
+                                                               (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
+                                                               &UpdatedBootOptionCount,
+                                                               &UpdatedBootOptions);
+    if (!EFI_ERROR (Status)) {
+      EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
+      BootOptions = UpdatedBootOptions;
+      BootOptionCount = UpdatedBootOptionCount;
+    }
+
+    //
+    // Call platform specific override to remove invalid boot options from NV
+    //
+    Status = PlatformBootOptions->RemoveInvalidPlatformNvBootOptions ((CONST UINTN)NvBootOptionCount,
+                                                                      (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)NvBootOptions,
+                                                                      &UpdatedBootOptionCount,
+                                                                      &UpdatedBootOptions);
+    if (!EFI_ERROR (Status)) {
+      EfiBootManagerFreeLoadOptions (NvBootOptions, NvBootOptionCount);
+      NvBootOptions = UpdatedBootOptions;
+      NvBootOptionCount = UpdatedBootOptionCount;
+    }
+  }
+
+  //
   // Remove invalid EFI boot options from NV
   //
   for (Index = 0; Index < NvBootOptionCount; Index++) {
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 027eb25..1f65860 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -1,6 +1,7 @@
 /** @file
   BDS library definition, include the file and data structure
 
+Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
 Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -41,6 +42,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Protocol/VariableLock.h>
 #include <Protocol/RamDisk.h>
 #include <Protocol/DeferredImageLoad.h>
+#include <Protocol/PlatformBootOptions.h>
 
 #include <Guid/MemoryTypeInformation.h>
 #include <Guid/FileInfo.h>
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index ed6b467..d839252 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -5,6 +5,7 @@
 #  manipulation, hotkey registration, UEFI boot, connect/disconnect, console
 #  manipulation, driver health checking and etc.
 #
+#  Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
 #  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -107,6 +108,7 @@
   gEfiFormBrowser2ProtocolGuid                  ## SOMETIMES_CONSUMES
   gEfiRamDiskProtocolGuid                       ## SOMETIMES_CONSUMES
   gEfiDeferredImageLoadProtocolGuid             ## SOMETIMES_CONSUMES
+  gEdkiiPlatformBootOptionsProtocolGuid         ## SOMETIMES_CONSUMES
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange      ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 41b9e70..bb9fa8b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -3,6 +3,7 @@
 # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs and library classes)
 # and libraries instances, which are used for those modules.
 #
+# Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
 # Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
 # Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
 # (C) Copyright 2016 - 2019 Hewlett Packard Enterprise Development LP<BR>
@@ -609,6 +610,9 @@
   ## Include/Protocol/PeCoffImageEmulator.h
   gEdkiiPeCoffImageEmulatorProtocolGuid = { 0x96f46153, 0x97a7, 0x4793, { 0xac, 0xc1, 0xfa, 0x19, 0xbf, 0x78, 0xea, 0x97 } }
 
+  ## Include/Protocol/PlatformBootOptions.h
+  gEdkiiPlatformBootOptionsProtocolGuid = { 0xaa17add4, 0x756c, 0x460d, { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e, 0x59 } }
+
 #
 # [Error.gEfiMdeModulePkgTokenSpaceGuid]
 #   0x80000001 | Invalid value provided.
-- 
2.7.4


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

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

Re: [edk2-devel] [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol
Posted by Ni, Ray 4 years, 3 months ago
Ashish,
I prefer EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL and could have two fields for this protocol for now:
Revision and RefreshAllBootOption (IN Options, IN OptionCount, OUT UpdatedOptions, OUT UpdatedOptionCount)
Usually EDKII puts Count in second and buffer in first.

The reason of using EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL as the new protocol name is in future
we could increase the revision and put more platform hook API in this protocol.

The reason of combining two APIs to one RefreshAllBootOption() is when I checked the code change below, I see
no need to separate them.

What do you think?

Thanks,
Ray

>    //
> +  // Locate Platform Boot Options Protocol
> +  //
> +  PlatformBootOptions = NULL;
> +  Status = gBS->LocateProtocol (&gEdkiiPlatformBootOptionsProtocolGuid,
> +                                NULL,
> +                                (VOID **)&PlatformBootOptions);
> +  if (!EFI_ERROR (Status)) {
> +    //
> +    // If found, call platform specific overrides to auto enumerated
> +    // boot options.
> +    //
> +    Status = PlatformBootOptions->OverridePlatformBootOptions ((CONST UINTN)BootOptionCount,
> +                                                               (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
> +                                                               &UpdatedBootOptionCount,
> +                                                               &UpdatedBootOptions);
> +    if (!EFI_ERROR (Status)) {
> +      EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> +      BootOptions = UpdatedBootOptions;
> +      BootOptionCount = UpdatedBootOptionCount;
> +    }
> +
> +    //
> +    // Call platform specific override to remove invalid boot options from NV
> +    //
> +    Status = PlatformBootOptions->RemoveInvalidPlatformNvBootOptions ((CONST UINTN)NvBootOptionCount,
> +                                                                      (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)NvBootOptions,
> +                                                                      &UpdatedBootOptionCount,
> +                                                                      &UpdatedBootOptions);
> +    if (!EFI_ERROR (Status)) {
> +      EfiBootManagerFreeLoadOptions (NvBootOptions, NvBootOptionCount);
> +      NvBootOptions = UpdatedBootOptions;
> +      NvBootOptionCount = UpdatedBootOptionCount;
> +    }
> +  }
> +


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

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

Re: [edk2-devel] [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol
Posted by Ashish Singhal 4 years, 3 months ago
Ray,

I did not name the protocol this way because EmbeddedPkg already describes a protocol with a similar name that is PLATFORM_BOOT_MANAGER_PROTOCOL. If you think we can still go ahead with new protocol named EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL, I can make the necessary changes.

I agree with your suggestion about unifying both functions into a single one as well.

Thanks
Ashish

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Tuesday, December 17, 2019 5:55 PM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: RE: [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol

External email: Use caution opening links or attachments


Ashish,
I prefer EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL and could have two fields for this protocol for now:
Revision and RefreshAllBootOption (IN Options, IN OptionCount, OUT UpdatedOptions, OUT UpdatedOptionCount) Usually EDKII puts Count in second and buffer in first.

The reason of using EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL as the new protocol name is in future we could increase the revision and put more platform hook API in this protocol.

The reason of combining two APIs to one RefreshAllBootOption() is when I checked the code change below, I see no need to separate them.

What do you think?

Thanks,
Ray

>    //
> +  // Locate Platform Boot Options Protocol  //  PlatformBootOptions = 
> + NULL;  Status = gBS->LocateProtocol 
> + (&gEdkiiPlatformBootOptionsProtocolGuid,
> +                                NULL,
> +                                (VOID **)&PlatformBootOptions);  if 
> + (!EFI_ERROR (Status)) {
> +    //
> +    // If found, call platform specific overrides to auto enumerated
> +    // boot options.
> +    //
> +    Status = PlatformBootOptions->OverridePlatformBootOptions ((CONST UINTN)BootOptionCount,
> +                                                               (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
> +                                                               &UpdatedBootOptionCount,
> +                                                               &UpdatedBootOptions);
> +    if (!EFI_ERROR (Status)) {
> +      EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> +      BootOptions = UpdatedBootOptions;
> +      BootOptionCount = UpdatedBootOptionCount;
> +    }
> +
> +    //
> +    // Call platform specific override to remove invalid boot options from NV
> +    //
> +    Status = PlatformBootOptions->RemoveInvalidPlatformNvBootOptions ((CONST UINTN)NvBootOptionCount,
> +                                                                      (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)NvBootOptions,
> +                                                                      &UpdatedBootOptionCount,
> +                                                                      &UpdatedBootOptions);
> +    if (!EFI_ERROR (Status)) {
> +      EfiBootManagerFreeLoadOptions (NvBootOptions, NvBootOptionCount);
> +      NvBootOptions = UpdatedBootOptions;
> +      NvBootOptionCount = UpdatedBootOptionCount;
> +    }
> +  }
> +

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

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

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

Re: [edk2-devel] [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol
Posted by Ni, Ray 4 years, 3 months ago
I am ok with adding EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL.

> -----Original Message-----
> From: Ashish Singhal <ashishsingha@nvidia.com>
> Sent: Wednesday, December 18, 2019 12:22 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: RE: [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol
> 
> Ray,
> 
> I did not name the protocol this way because EmbeddedPkg already describes a protocol with a similar name that is
> PLATFORM_BOOT_MANAGER_PROTOCOL. If you think we can still go ahead with new protocol named
> EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL, I can make the necessary changes.
> 
> I agree with your suggestion about unifying both functions into a single one as well.
> 
> Thanks
> Ashish
> 
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, December 17, 2019 5:55 PM
> To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Subject: RE: [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol
> 
> External email: Use caution opening links or attachments
> 
> 
> Ashish,
> I prefer EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL and could have two fields for this protocol for now:
> Revision and RefreshAllBootOption (IN Options, IN OptionCount, OUT UpdatedOptions, OUT UpdatedOptionCount)
> Usually EDKII puts Count in second and buffer in first.
> 
> The reason of using EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL as the new protocol name is in future we could
> increase the revision and put more platform hook API in this protocol.
> 
> The reason of combining two APIs to one RefreshAllBootOption() is when I checked the code change below, I see no need
> to separate them.
> 
> What do you think?
> 
> Thanks,
> Ray
> 
> >    //
> > +  // Locate Platform Boot Options Protocol  //  PlatformBootOptions =
> > + NULL;  Status = gBS->LocateProtocol
> > + (&gEdkiiPlatformBootOptionsProtocolGuid,
> > +                                NULL,
> > +                                (VOID **)&PlatformBootOptions);  if
> > + (!EFI_ERROR (Status)) {
> > +    //
> > +    // If found, call platform specific overrides to auto enumerated
> > +    // boot options.
> > +    //
> > +    Status = PlatformBootOptions->OverridePlatformBootOptions ((CONST UINTN)BootOptionCount,
> > +                                                               (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
> > +                                                               &UpdatedBootOptionCount,
> > +                                                               &UpdatedBootOptions);
> > +    if (!EFI_ERROR (Status)) {
> > +      EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> > +      BootOptions = UpdatedBootOptions;
> > +      BootOptionCount = UpdatedBootOptionCount;
> > +    }
> > +
> > +    //
> > +    // Call platform specific override to remove invalid boot options from NV
> > +    //
> > +    Status = PlatformBootOptions->RemoveInvalidPlatformNvBootOptions ((CONST UINTN)NvBootOptionCount,
> > +                                                                      (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)NvBootOptions,
> > +                                                                      &UpdatedBootOptionCount,
> > +                                                                      &UpdatedBootOptions);
> > +    if (!EFI_ERROR (Status)) {
> > +      EfiBootManagerFreeLoadOptions (NvBootOptions, NvBootOptionCount);
> > +      NvBootOptions = UpdatedBootOptions;
> > +      NvBootOptionCount = UpdatedBootOptionCount;
> > +    }
> > +  }
> > +
> 
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information.  Any unauthorized review, use, disclosure or distribution
> is prohibited.  If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------

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

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

Re: [edk2-devel] [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol
Posted by Ashish Singhal 4 years, 3 months ago
I have submitted a patch version 4.

Thanks
Ashish

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Wednesday, December 18, 2019 1:43 AM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: RE: [PATCH v3] MdeModulePkg: Add Platform Boot Options Protocol

External email: Use caution opening links or attachments


I am ok with adding EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL.

> -----Original Message-----
> From: Ashish Singhal <ashishsingha@nvidia.com>
> Sent: Wednesday, December 18, 2019 12:22 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Wang, Jian J 
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao 
> <zhichao.gao@intel.com>
> Subject: RE: [PATCH v3] MdeModulePkg: Add Platform Boot Options 
> Protocol
>
> Ray,
>
> I did not name the protocol this way because EmbeddedPkg already 
> describes a protocol with a similar name that is 
> PLATFORM_BOOT_MANAGER_PROTOCOL. If you think we can still go ahead with new protocol named EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL, I can make the necessary changes.
>
> I agree with your suggestion about unifying both functions into a single one as well.
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, December 17, 2019 5:55 PM
> To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io; 
> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; 
> Gao, Zhichao <zhichao.gao@intel.com>
> Subject: RE: [PATCH v3] MdeModulePkg: Add Platform Boot Options 
> Protocol
>
> External email: Use caution opening links or attachments
>
>
> Ashish,
> I prefer EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL and could have two fields for this protocol for now:
> Revision and RefreshAllBootOption (IN Options, IN OptionCount, OUT 
> UpdatedOptions, OUT UpdatedOptionCount) Usually EDKII puts Count in second and buffer in first.
>
> The reason of using EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL as the new 
> protocol name is in future we could increase the revision and put more platform hook API in this protocol.
>
> The reason of combining two APIs to one RefreshAllBootOption() is when 
> I checked the code change below, I see no need to separate them.
>
> What do you think?
>
> Thanks,
> Ray
>
> >    //
> > +  // Locate Platform Boot Options Protocol  //  PlatformBootOptions 
> > + = NULL;  Status = gBS->LocateProtocol 
> > + (&gEdkiiPlatformBootOptionsProtocolGuid,
> > +                                NULL,
> > +                                (VOID **)&PlatformBootOptions);  if 
> > + (!EFI_ERROR (Status)) {
> > +    //
> > +    // If found, call platform specific overrides to auto enumerated
> > +    // boot options.
> > +    //
> > +    Status = PlatformBootOptions->OverridePlatformBootOptions ((CONST UINTN)BootOptionCount,
> > +                                                               (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
> > +                                                               &UpdatedBootOptionCount,
> > +                                                               &UpdatedBootOptions);
> > +    if (!EFI_ERROR (Status)) {
> > +      EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> > +      BootOptions = UpdatedBootOptions;
> > +      BootOptionCount = UpdatedBootOptionCount;
> > +    }
> > +
> > +    //
> > +    // Call platform specific override to remove invalid boot options from NV
> > +    //
> > +    Status = PlatformBootOptions->RemoveInvalidPlatformNvBootOptions ((CONST UINTN)NvBootOptionCount,
> > +                                                                      (CONST EFI_BOOT_MANAGER_LOAD_OPTION *)NvBootOptions,
> > +                                                                      &UpdatedBootOptionCount,
> > +                                                                      &UpdatedBootOptions);
> > +    if (!EFI_ERROR (Status)) {
> > +      EfiBootManagerFreeLoadOptions (NvBootOptions, NvBootOptionCount);
> > +      NvBootOptions = UpdatedBootOptions;
> > +      NvBootOptionCount = UpdatedBootOptionCount;
> > +    }
> > +  }
> > +
>
> ----------------------------------------------------------------------
> ------------- This email message is for the sole use of the intended 
> recipient(s) and may contain confidential information.  Any 
> unauthorized review, use, disclosure or distribution is prohibited.  
> If you are not the intended recipient, please contact the sender by 
> reply email and destroy all copies of the original message.
> ----------------------------------------------------------------------
> -------------

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

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