* [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
@ 2019-12-18 19:10 Ashish Singhal
2019-12-19 1:59 ` Ni, Ray
2019-12-23 1:46 ` Ni, Ray
0 siblings, 2 replies; 12+ messages in thread
From: Ashish Singhal @ 2019-12-18 19:10 UTC (permalink / raw)
To: devel, jian.j.wang, hao.a.wu, zhichao.gao, ray.ni; +Cc: Ashish Singhal
Add edk2 platform boot manager protocol which would have platform
specific refreshes to the auto enumerated as well as NV boot options
for the platform.
Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
---
.../Include/Protocol/PlatformBootManager.h | 82 ++++++++++++++++++++++
MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 41 +++++++++--
.../Library/UefiBootManagerLib/InternalBm.h | 2 +
.../UefiBootManagerLib/UefiBootManagerLib.inf | 2 +
MdeModulePkg/MdeModulePkg.dec | 4 ++
5 files changed, 124 insertions(+), 7 deletions(-)
create mode 100644 MdeModulePkg/Include/Protocol/PlatformBootManager.h
diff --git a/MdeModulePkg/Include/Protocol/PlatformBootManager.h b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
new file mode 100644
index 0000000..ec32215
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
@@ -0,0 +1,82 @@
+/** @file
+
+ Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
+#define __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
+
+#include <Library/UefiBootManagerLib.h>
+
+//
+// Platform Boot Manager Protocol GUID value
+//
+#define EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL_GUID \
+ { \
+ 0xaa17add4, 0x756c, 0x460d, { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e, 0x59 } \
+ }
+
+//
+// Protocol interface structure
+//
+typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL PLATFORM_BOOT_MANAGER_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_MANAGER_PROTOCOL_REVISION 0x00000001
+
+//
+// Function Prototypes
+//
+
+/*
+ This function allows platform to refresh all boot options specific to the platform. Within
+ this function, platform can make modifications to the auto enumerated platform boot options
+ as well as NV boot options.
+
+ @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[in const] BootOptionsCount The number of elements in BootOptions.
+
+ @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 REFRESH_ALL_BOOT_OPTIONS
+ and would be freed by caller after consuming it.
+
+ @param[out] UpdatedBootOptionsCount The number of elements in UpdatedBootOptions.
+
+
+ @retval EFI_SUCCESS Platform refresh 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 *REFRESH_ALL_BOOT_OPTIONS) (
+ IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions,
+ IN CONST UINTN BootOptionsCount,
+ OUT EFI_BOOT_MANAGER_LOAD_OPTION **UpdatedBootOptions,
+ OUT UINTN *UpdatedBootOptionsCount
+ );
+
+struct _PLATFORM_BOOT_MANAGER_PROTOCOL {
+ UINT64 Revision;
+ REFRESH_ALL_BOOT_OPTIONS RefreshAllBootOptions;
+};
+
+extern EFI_GUID gEdkiiPlatformBootManagerProtocolGuid;
+
+#endif /* __PLATFORM_BOOT_MANAGER_PROTOCOL_H__ */
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 760d764..8b9a76d 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_MANAGER_PROTOCOL *PlatformBootManager;
//
// Optionally refresh the legacy boot option
@@ -2273,7 +2277,6 @@ EfiBootManagerRefreshAllBootOption (
}
BootOptions = BmEnumerateBootOptions (&BootOptionCount);
- NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount, LoadOptionTypeBoot);
//
// Mark the boot option as added by BDS by setting OptionalData to a special GUID
@@ -2284,6 +2287,30 @@ EfiBootManagerRefreshAllBootOption (
}
//
+ // Locate Platform Boot Options Protocol
+ //
+ Status = gBS->LocateProtocol (&gEdkiiPlatformBootManagerProtocolGuid,
+ NULL,
+ (VOID **)&PlatformBootManager);
+ if (!EFI_ERROR (Status)) {
+ //
+ // If found, call platform specific refresh to all auto enumerated and NV
+ // boot options.
+ //
+ Status = PlatformBootManager->RefreshAllBootOptions ((CONST EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
+ (CONST UINTN)BootOptionCount,
+ &UpdatedBootOptions,
+ &UpdatedBootOptionCount);
+ if (!EFI_ERROR (Status)) {
+ EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
+ BootOptions = UpdatedBootOptions;
+ BootOptionCount = UpdatedBootOptionCount;
+ }
+ }
+
+ NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount, LoadOptionTypeBoot);
+
+ //
// 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..ac866ac 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/PlatformBootManager.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..cf59086 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
+ gEdkiiPlatformBootManagerProtocolGuid ## SOMETIMES_CONSUMES
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 41b9e70..cc238e9 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/PlatformBootManager.h
+ gEdkiiPlatformBootManagerProtocolGuid = { 0xaa17add4, 0x756c, 0x460d, { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e, 0x59 } }
+
#
# [Error.gEfiMdeModulePkgTokenSpaceGuid]
# 0x80000001 | Invalid value provided.
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-18 19:10 [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol Ashish Singhal
@ 2019-12-19 1:59 ` Ni, Ray
2019-12-19 14:22 ` [edk2-devel] " Wang, Sunny (HPS SW)
2019-12-23 1:46 ` Ni, Ray
1 sibling, 1 reply; 12+ messages in thread
From: Ni, Ray @ 2019-12-19 1:59 UTC (permalink / raw)
To: Wang, Jian J, Wu, Hao A, Gao, Zhichao, Kinney, Michael D,
'Andrew Fish (afish@apple.com)'
Cc: devel@edk2.groups.io, Ashish Singhal
All,
The new EDKII Platform Boot Manager protocol provides a platform hook to solve below problem.
Can you please review and think about:
1. Is it a proper solution to the problem?
2. Is the new protocol/function name proper?
3. Are the parameters in the function proper?
**Problem:
Platform requires certain BlockIo/SimpleFileSystem/LoadFile instances don't cause Boot#### created. It's a need of platform customization.
**Details:
Boot#### for BlockIo/SimpleFileSystem/LoadFile are created by API EfiBootMangerRefreshAllBootOptions(). There are 2 places that call this API:
1. Platform Boot Manager calls the API (usually in the full configuration boot path)
2. UiApp calls the API when entering "Boot Manager" page and "Boot Maintenance Manager" page.
Platform can change Platform Boot Manager library to remove the unneeded Boot#### in case #1.
But platform has no way to remove the Boot#### created in case #2 .
Thanks,
Ray
> -----Original Message-----
> From: Ashish Singhal <ashishsingha@nvidia.com>
> Sent: Thursday, December 19, 2019 3:11 AM
> To: 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>; Ni, Ray
> <ray.ni@intel.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com>
> Subject: [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager
> Protocol
>
> Add edk2 platform boot manager protocol which would have platform
> specific refreshes to the auto enumerated as well as NV boot options
> for the platform.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> .../Include/Protocol/PlatformBootManager.h | 82
> ++++++++++++++++++++++
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 41 +++++++++--
> .../Library/UefiBootManagerLib/InternalBm.h | 2 +
> .../UefiBootManagerLib/UefiBootManagerLib.inf | 2 +
> MdeModulePkg/MdeModulePkg.dec | 4 ++
> 5 files changed, 124 insertions(+), 7 deletions(-)
> create mode 100644
> MdeModulePkg/Include/Protocol/PlatformBootManager.h
>
> diff --git a/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> new file mode 100644
> index 0000000..ec32215
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> @@ -0,0 +1,82 @@
> +/** @file
> +
> + Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
> +#define __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
> +
> +#include <Library/UefiBootManagerLib.h>
> +
> +//
> +// Platform Boot Manager Protocol GUID value
> +//
> +#define EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL_GUID \
> + { \
> + 0xaa17add4, 0x756c, 0x460d, { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e,
> 0x59 } \
> + }
> +
> +//
> +// Protocol interface structure
> +//
> +typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL
> PLATFORM_BOOT_MANAGER_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_MANAGER_PROTOCOL_REVISION
> 0x00000001
> +
> +//
> +// Function Prototypes
> +//
> +
> +/*
> + This function allows platform to refresh all boot options specific to the
> platform. Within
> + this function, platform can make modifications to the auto enumerated
> platform boot options
> + as well as NV boot options.
> +
> + @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[in const] BootOptionsCount The number of elements in
> BootOptions.
> +
> + @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
> REFRESH_ALL_BOOT_OPTIONS
> + and would be freed by caller after consuming it.
> +
> + @param[out] UpdatedBootOptionsCount The number of elements in
> UpdatedBootOptions.
> +
> +
> + @retval EFI_SUCCESS Platform refresh 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 *REFRESH_ALL_BOOT_OPTIONS) (
> + IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions,
> + IN CONST UINTN BootOptionsCount,
> + OUT EFI_BOOT_MANAGER_LOAD_OPTION **UpdatedBootOptions,
> + OUT UINTN *UpdatedBootOptionsCount
> + );
> +
> +struct _PLATFORM_BOOT_MANAGER_PROTOCOL {
> + UINT64 Revision;
> + REFRESH_ALL_BOOT_OPTIONS RefreshAllBootOptions;
> +};
> +
> +extern EFI_GUID gEdkiiPlatformBootManagerProtocolGuid;
> +
> +#endif /* __PLATFORM_BOOT_MANAGER_PROTOCOL_H__ */
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 760d764..8b9a76d 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_MANAGER_PROTOCOL *PlatformBootManager;
>
> //
> // Optionally refresh the legacy boot option
> @@ -2273,7 +2277,6 @@ EfiBootManagerRefreshAllBootOption (
> }
>
> BootOptions = BmEnumerateBootOptions (&BootOptionCount);
> - NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount,
> LoadOptionTypeBoot);
>
> //
> // Mark the boot option as added by BDS by setting OptionalData to a
> special GUID
> @@ -2284,6 +2287,30 @@ EfiBootManagerRefreshAllBootOption (
> }
>
> //
> + // Locate Platform Boot Options Protocol
> + //
> + Status = gBS->LocateProtocol (&gEdkiiPlatformBootManagerProtocolGuid,
> + NULL,
> + (VOID **)&PlatformBootManager);
> + if (!EFI_ERROR (Status)) {
> + //
> + // If found, call platform specific refresh to all auto enumerated and NV
> + // boot options.
> + //
> + Status = PlatformBootManager->RefreshAllBootOptions ((CONST
> EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
> + (CONST UINTN)BootOptionCount,
> + &UpdatedBootOptions,
> + &UpdatedBootOptionCount);
> + if (!EFI_ERROR (Status)) {
> + EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> + BootOptions = UpdatedBootOptions;
> + BootOptionCount = UpdatedBootOptionCount;
> + }
> + }
> +
> + NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount,
> LoadOptionTypeBoot);
> +
> + //
> // 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..ac866ac 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/PlatformBootManager.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..cf59086 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
> + gEdkiiPlatformBootManagerProtocolGuid ## SOMETIMES_CONSUMES
>
> [Pcd]
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC
> hange ## SOMETIMES_CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 41b9e70..cc238e9 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/PlatformBootManager.h
> + gEdkiiPlatformBootManagerProtocolGuid = { 0xaa17add4, 0x756c, 0x460d,
> { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e, 0x59 } }
> +
> #
> # [Error.gEfiMdeModulePkgTokenSpaceGuid]
> # 0x80000001 | Invalid value provided.
> --
> 2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-19 1:59 ` Ni, Ray
@ 2019-12-19 14:22 ` Wang, Sunny (HPS SW)
2019-12-19 16:38 ` Ashish Singhal
0 siblings, 1 reply; 12+ messages in thread
From: Wang, Sunny (HPS SW) @ 2019-12-19 14:22 UTC (permalink / raw)
To: devel@edk2.groups.io, ray.ni@intel.com, Wang, Jian J, Wu, Hao A,
Gao, Zhichao, Kinney, Michael D,
'Andrew Fish (afish@apple.com)'
Cc: Ashish Singhal, Wang, Sunny (HPS SW), Spottswood, Jason
1. Is it a proper solution to the problem?
Yes, it already solved my concern discussed in the other email.
2. Is the new protocol/function name proper?
Yes, but I'm not good at naming. We may need others' feedback. :)
3. Are the parameters in the function proper?
How about we only have two parameters as below to simplify this code change?
typedef
EFI_STATUS
(EFIAPI *REFRESH_ALL_BOOT_OPTIONS) (
IN OUT EFI_BOOT_MANAGER_LOAD_OPTION **BootOptions,
IN OUT UINTN *BootOptionsCount,
);
In addition, I have one more suggestion about the structure name inline.
Besides these two comments, everything else looks good to me.
Regards,
Sunny Wang
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni, Ray
Sent: Thursday, December 19, 2019 9:59 AM
To: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>
Subject: Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
All,
The new EDKII Platform Boot Manager protocol provides a platform hook to solve below problem.
Can you please review and think about:
1. Is it a proper solution to the problem?
2. Is the new protocol/function name proper?
3. Are the parameters in the function proper?
**Problem:
Platform requires certain BlockIo/SimpleFileSystem/LoadFile instances don't cause Boot#### created. It's a need of platform customization.
**Details:
Boot#### for BlockIo/SimpleFileSystem/LoadFile are created by API EfiBootMangerRefreshAllBootOptions(). There are 2 places that call this API:
1. Platform Boot Manager calls the API (usually in the full configuration boot path)
2. UiApp calls the API when entering "Boot Manager" page and "Boot Maintenance Manager" page.
Platform can change Platform Boot Manager library to remove the unneeded Boot#### in case #1.
But platform has no way to remove the Boot#### created in case #2 .
Thanks,
Ray
> -----Original Message-----
> From: Ashish Singhal <ashishsingha@nvidia.com>
> Sent: Thursday, December 19, 2019 3:11 AM
> To: 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>; Ni,
> Ray <ray.ni@intel.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com>
> Subject: [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager
> Protocol
>
> Add edk2 platform boot manager protocol which would have platform
> specific refreshes to the auto enumerated as well as NV boot options
> for the platform.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> .../Include/Protocol/PlatformBootManager.h | 82
> ++++++++++++++++++++++
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 41 +++++++++--
> .../Library/UefiBootManagerLib/InternalBm.h | 2 +
> .../UefiBootManagerLib/UefiBootManagerLib.inf | 2 +
> MdeModulePkg/MdeModulePkg.dec | 4 ++
> 5 files changed, 124 insertions(+), 7 deletions(-) create mode
> 100644 MdeModulePkg/Include/Protocol/PlatformBootManager.h
>
> diff --git a/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> new file mode 100644
> index 0000000..ec32215
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> @@ -0,0 +1,82 @@
> +/** @file
> +
> + Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
> +#define __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
> +
> +#include <Library/UefiBootManagerLib.h>
> +
> +//
> +// Platform Boot Manager Protocol GUID value // #define
> +EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL_GUID \
> + { \
> + 0xaa17add4, 0x756c, 0x460d, { 0x94, 0xb8, 0x43, 0x88, 0xd7,
> +0xfb, 0x3e,
> 0x59 } \
> + }
> +
> +//
> +// Protocol interface structure
> +//
> +typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL
> PLATFORM_BOOT_MANAGER_PROTOCOL;
For being consistent with other EDKII protocols like EDKII_PLATFORM_LOGO_PROTOCOL , how about we update it to the following?
typedef struct _EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL EDKII_PLATFORM_BOOT_MANAGER_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_MANAGER_PROTOCOL_REVISION
> 0x00000001
> +
> +//
> +// Function Prototypes
> +//
> +
> +/*
> + This function allows platform to refresh all boot options specific
> +to the
> platform. Within
> + this function, platform can make modifications to the auto
> + enumerated
> platform boot options
> + as well as NV boot options.
> +
> + @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[in const] BootOptionsCount The number of elements in
> BootOptions.
> +
> + @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
> REFRESH_ALL_BOOT_OPTIONS
> + and would be freed by caller after consuming it.
> +
> + @param[out] UpdatedBootOptionsCount The number of elements in
> UpdatedBootOptions.
> +
> +
> + @retval EFI_SUCCESS Platform refresh 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 *REFRESH_ALL_BOOT_OPTIONS) (
> + IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions,
> + IN CONST UINTN BootOptionsCount,
> + OUT EFI_BOOT_MANAGER_LOAD_OPTION **UpdatedBootOptions,
> + OUT UINTN *UpdatedBootOptionsCount
> + );
> +
> +struct _PLATFORM_BOOT_MANAGER_PROTOCOL {
> + UINT64 Revision;
> + REFRESH_ALL_BOOT_OPTIONS RefreshAllBootOptions; };
> +
> +extern EFI_GUID gEdkiiPlatformBootManagerProtocolGuid;
> +
> +#endif /* __PLATFORM_BOOT_MANAGER_PROTOCOL_H__ */
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 760d764..8b9a76d 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_MANAGER_PROTOCOL *PlatformBootManager;
>
> //
> // Optionally refresh the legacy boot option @@ -2273,7 +2277,6 @@
> EfiBootManagerRefreshAllBootOption (
> }
>
> BootOptions = BmEnumerateBootOptions (&BootOptionCount);
> - NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount,
> LoadOptionTypeBoot);
>
> //
> // Mark the boot option as added by BDS by setting OptionalData to
> a special GUID @@ -2284,6 +2287,30 @@
> EfiBootManagerRefreshAllBootOption (
> }
>
> //
> + // Locate Platform Boot Options Protocol // Status =
> + gBS->LocateProtocol (&gEdkiiPlatformBootManagerProtocolGuid,
> + NULL,
> + (VOID **)&PlatformBootManager); if
> + (!EFI_ERROR (Status)) {
> + //
> + // If found, call platform specific refresh to all auto enumerated and NV
> + // boot options.
> + //
> + Status = PlatformBootManager->RefreshAllBootOptions ((CONST
> EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
> + (CONST UINTN)BootOptionCount,
> + &UpdatedBootOptions,
> + &UpdatedBootOptionCount);
> + if (!EFI_ERROR (Status)) {
> + EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> + BootOptions = UpdatedBootOptions;
> + BootOptionCount = UpdatedBootOptionCount;
> + }
> + }
> +
> + NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount,
> LoadOptionTypeBoot);
> +
> + //
> // 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..ac866ac 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/PlatformBootManager.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..cf59086 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
> + gEdkiiPlatformBootManagerProtocolGuid ## SOMETIMES_CONSUMES
>
> [Pcd]
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC
> hange ## SOMETIMES_CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index 41b9e70..cc238e9 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/PlatformBootManager.h
> + gEdkiiPlatformBootManagerProtocolGuid = { 0xaa17add4, 0x756c,
> + 0x460d,
> { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e, 0x59 } }
> +
> #
> # [Error.gEfiMdeModulePkgTokenSpaceGuid]
> # 0x80000001 | Invalid value provided.
> --
> 2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-19 14:22 ` [edk2-devel] " Wang, Sunny (HPS SW)
@ 2019-12-19 16:38 ` Ashish Singhal
2019-12-20 11:28 ` Wang, Sunny (HPS SW)
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Singhal @ 2019-12-19 16:38 UTC (permalink / raw)
To: Wang, Sunny (HPS SW), devel@edk2.groups.io, ray.ni@intel.com,
Wang, Jian J, Wu, Hao A, Gao, Zhichao, Kinney, Michael D,
'Andrew Fish (afish@apple.com)'
Cc: Spottswood, Jason
Hi Sunny,
I thought about keeping the arguments the way you suggested but then decided against it so that the auto enumerated boot options list is not tampered with in case the function implementation hits an error in between. This way, if the function returns an error, we can use the list we passed in and continue boot.
I am OK with any suggested name changes.
Thanks
Ashish
-----Original Message-----
From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Sent: Thursday, December 19, 2019 7:23 AM
To: devel@edk2.groups.io; ray.ni@intel.com; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Sunny (HPS SW) <sunnywang@hpe.com>; Spottswood, Jason <jason.spottswood@hpe.com>
Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
External email: Use caution opening links or attachments
1. Is it a proper solution to the problem?
Yes, it already solved my concern discussed in the other email.
2. Is the new protocol/function name proper?
Yes, but I'm not good at naming. We may need others' feedback. :) 3. Are the parameters in the function proper?
How about we only have two parameters as below to simplify this code change?
typedef
EFI_STATUS
(EFIAPI *REFRESH_ALL_BOOT_OPTIONS) (
IN OUT EFI_BOOT_MANAGER_LOAD_OPTION **BootOptions,
IN OUT UINTN *BootOptionsCount,
);
In addition, I have one more suggestion about the structure name inline.
Besides these two comments, everything else looks good to me.
Regards,
Sunny Wang
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni, Ray
Sent: Thursday, December 19, 2019 9:59 AM
To: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>
Subject: Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
All,
The new EDKII Platform Boot Manager protocol provides a platform hook to solve below problem.
Can you please review and think about:
1. Is it a proper solution to the problem?
2. Is the new protocol/function name proper?
3. Are the parameters in the function proper?
**Problem:
Platform requires certain BlockIo/SimpleFileSystem/LoadFile instances don't cause Boot#### created. It's a need of platform customization.
**Details:
Boot#### for BlockIo/SimpleFileSystem/LoadFile are created by API EfiBootMangerRefreshAllBootOptions(). There are 2 places that call this API:
1. Platform Boot Manager calls the API (usually in the full configuration boot path)
2. UiApp calls the API when entering "Boot Manager" page and "Boot Maintenance Manager" page.
Platform can change Platform Boot Manager library to remove the unneeded Boot#### in case #1.
But platform has no way to remove the Boot#### created in case #2 .
Thanks,
Ray
> -----Original Message-----
> From: Ashish Singhal <ashishsingha@nvidia.com>
> Sent: Thursday, December 19, 2019 3:11 AM
> To: 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>; Ni,
> Ray <ray.ni@intel.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com>
> Subject: [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager
> Protocol
>
> Add edk2 platform boot manager protocol which would have platform
> specific refreshes to the auto enumerated as well as NV boot options
> for the platform.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> .../Include/Protocol/PlatformBootManager.h | 82
> ++++++++++++++++++++++
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 41 +++++++++--
> .../Library/UefiBootManagerLib/InternalBm.h | 2 +
> .../UefiBootManagerLib/UefiBootManagerLib.inf | 2 +
> MdeModulePkg/MdeModulePkg.dec | 4 ++
> 5 files changed, 124 insertions(+), 7 deletions(-) create mode
> 100644 MdeModulePkg/Include/Protocol/PlatformBootManager.h
>
> diff --git a/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> new file mode 100644
> index 0000000..ec32215
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> @@ -0,0 +1,82 @@
> +/** @file
> +
> + Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
> +#define __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
> +
> +#include <Library/UefiBootManagerLib.h>
> +
> +//
> +// Platform Boot Manager Protocol GUID value // #define
> +EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL_GUID \
> + { \
> + 0xaa17add4, 0x756c, 0x460d, { 0x94, 0xb8, 0x43, 0x88, 0xd7,
> +0xfb, 0x3e,
> 0x59 } \
> + }
> +
> +//
> +// Protocol interface structure
> +//
> +typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL
> PLATFORM_BOOT_MANAGER_PROTOCOL;
For being consistent with other EDKII protocols like EDKII_PLATFORM_LOGO_PROTOCOL , how about we update it to the following?
typedef struct _EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL EDKII_PLATFORM_BOOT_MANAGER_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_MANAGER_PROTOCOL_REVISION
> 0x00000001
> +
> +//
> +// Function Prototypes
> +//
> +
> +/*
> + This function allows platform to refresh all boot options specific
> +to the
> platform. Within
> + this function, platform can make modifications to the auto
> + enumerated
> platform boot options
> + as well as NV boot options.
> +
> + @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[in const] BootOptionsCount The number of elements in
> BootOptions.
> +
> + @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
> REFRESH_ALL_BOOT_OPTIONS
> + and would be freed by caller after consuming it.
> +
> + @param[out] UpdatedBootOptionsCount The number of elements in
> UpdatedBootOptions.
> +
> +
> + @retval EFI_SUCCESS Platform refresh 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 *REFRESH_ALL_BOOT_OPTIONS) (
> + IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions,
> + IN CONST UINTN BootOptionsCount,
> + OUT EFI_BOOT_MANAGER_LOAD_OPTION **UpdatedBootOptions,
> + OUT UINTN *UpdatedBootOptionsCount
> + );
> +
> +struct _PLATFORM_BOOT_MANAGER_PROTOCOL {
> + UINT64 Revision;
> + REFRESH_ALL_BOOT_OPTIONS RefreshAllBootOptions; };
> +
> +extern EFI_GUID gEdkiiPlatformBootManagerProtocolGuid;
> +
> +#endif /* __PLATFORM_BOOT_MANAGER_PROTOCOL_H__ */
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 760d764..8b9a76d 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_MANAGER_PROTOCOL *PlatformBootManager;
>
> //
> // Optionally refresh the legacy boot option @@ -2273,7 +2277,6 @@
> EfiBootManagerRefreshAllBootOption (
> }
>
> BootOptions = BmEnumerateBootOptions (&BootOptionCount);
> - NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount,
> LoadOptionTypeBoot);
>
> //
> // Mark the boot option as added by BDS by setting OptionalData to
> a special GUID @@ -2284,6 +2287,30 @@
> EfiBootManagerRefreshAllBootOption (
> }
>
> //
> + // Locate Platform Boot Options Protocol // Status =
> + gBS->LocateProtocol (&gEdkiiPlatformBootManagerProtocolGuid,
> + NULL,
> + (VOID **)&PlatformBootManager); if
> + (!EFI_ERROR (Status)) {
> + //
> + // If found, call platform specific refresh to all auto enumerated and NV
> + // boot options.
> + //
> + Status = PlatformBootManager->RefreshAllBootOptions ((CONST
> EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
> + (CONST UINTN)BootOptionCount,
> + &UpdatedBootOptions,
> + &UpdatedBootOptionCount);
> + if (!EFI_ERROR (Status)) {
> + EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> + BootOptions = UpdatedBootOptions;
> + BootOptionCount = UpdatedBootOptionCount;
> + }
> + }
> +
> + NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount,
> LoadOptionTypeBoot);
> +
> + //
> // 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..ac866ac 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/PlatformBootManager.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..cf59086 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
> + gEdkiiPlatformBootManagerProtocolGuid ## SOMETIMES_CONSUMES
>
> [Pcd]
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC
> hange ## SOMETIMES_CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index 41b9e70..cc238e9 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/PlatformBootManager.h
> + gEdkiiPlatformBootManagerProtocolGuid = { 0xaa17add4, 0x756c,
> + 0x460d,
> { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e, 0x59 } }
> +
> #
> # [Error.gEfiMdeModulePkgTokenSpaceGuid]
> # 0x80000001 | Invalid value provided.
> --
> 2.7.4
-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-19 16:38 ` Ashish Singhal
@ 2019-12-20 11:28 ` Wang, Sunny (HPS SW)
2019-12-24 2:40 ` Ni, Ray
0 siblings, 1 reply; 12+ messages in thread
From: Wang, Sunny (HPS SW) @ 2019-12-20 11:28 UTC (permalink / raw)
To: devel@edk2.groups.io, ashishsingha@nvidia.com, ray.ni@intel.com,
Wang, Jian J, Wu, Hao A, Gao, Zhichao, Kinney, Michael D,
'Andrew Fish (afish@apple.com)'
Cc: Spottswood, Jason, Wang, Sunny (HPS SW)
Good point. The way you used is more robust. It can cover a mistake in function's error handling. Thanks for clarifying this, Ashish.
In addition, the other naming suggestion just comes to mind. How about we rename the function to a more generic one (based on location) like AfterEnumerateBootOptions or a more specific one like RefreshEnumeratedBootOptions? In the future, we may add the other hook function in the EfiBootManagerRefreshAllBootOption to deal with the boot options that are not created by BmEnumerateBootOptions. In this case (two hook functions in EfiBootManagerRefreshAllBootOption), the original function name "RefreshAllBootOptions" may cause some confusion.
Regards,
Sunny Wang
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, December 20, 2019 12:38 AM
To: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; devel@edk2.groups.io; ray.ni@intel.com; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Spottswood, Jason <jason.spottswood@hpe.com>
Subject: Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
Hi Sunny,
I thought about keeping the arguments the way you suggested but then decided against it so that the auto enumerated boot options list is not tampered with in case the function implementation hits an error in between. This way, if the function returns an error, we can use the list we passed in and continue boot.
I am OK with any suggested name changes.
Thanks
Ashish
-----Original Message-----
From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Sent: Thursday, December 19, 2019 7:23 AM
To: devel@edk2.groups.io; ray.ni@intel.com; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Ashish Singhal <ashishsingha@nvidia.com>; Wang, Sunny (HPS SW) <sunnywang@hpe.com>; Spottswood, Jason <jason.spottswood@hpe.com>
Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
External email: Use caution opening links or attachments
1. Is it a proper solution to the problem?
Yes, it already solved my concern discussed in the other email.
2. Is the new protocol/function name proper?
Yes, but I'm not good at naming. We may need others' feedback. :) 3. Are the parameters in the function proper?
How about we only have two parameters as below to simplify this code change?
typedef
EFI_STATUS
(EFIAPI *REFRESH_ALL_BOOT_OPTIONS) (
IN OUT EFI_BOOT_MANAGER_LOAD_OPTION **BootOptions,
IN OUT UINTN *BootOptionsCount,
);
In addition, I have one more suggestion about the structure name inline.
Besides these two comments, everything else looks good to me.
Regards,
Sunny Wang
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni, Ray
Sent: Thursday, December 19, 2019 9:59 AM
To: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>
Subject: Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
All,
The new EDKII Platform Boot Manager protocol provides a platform hook to solve below problem.
Can you please review and think about:
1. Is it a proper solution to the problem?
2. Is the new protocol/function name proper?
3. Are the parameters in the function proper?
**Problem:
Platform requires certain BlockIo/SimpleFileSystem/LoadFile instances don't cause Boot#### created. It's a need of platform customization.
**Details:
Boot#### for BlockIo/SimpleFileSystem/LoadFile are created by API EfiBootMangerRefreshAllBootOptions(). There are 2 places that call this API:
1. Platform Boot Manager calls the API (usually in the full configuration boot path)
2. UiApp calls the API when entering "Boot Manager" page and "Boot Maintenance Manager" page.
Platform can change Platform Boot Manager library to remove the unneeded Boot#### in case #1.
But platform has no way to remove the Boot#### created in case #2 .
Thanks,
Ray
> -----Original Message-----
> From: Ashish Singhal <ashishsingha@nvidia.com>
> Sent: Thursday, December 19, 2019 3:11 AM
> To: 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>; Ni,
> Ray <ray.ni@intel.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com>
> Subject: [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager
> Protocol
>
> Add edk2 platform boot manager protocol which would have platform
> specific refreshes to the auto enumerated as well as NV boot options
> for the platform.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> .../Include/Protocol/PlatformBootManager.h | 82
> ++++++++++++++++++++++
> MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 41 +++++++++--
> .../Library/UefiBootManagerLib/InternalBm.h | 2 +
> .../UefiBootManagerLib/UefiBootManagerLib.inf | 2 +
> MdeModulePkg/MdeModulePkg.dec | 4 ++
> 5 files changed, 124 insertions(+), 7 deletions(-) create mode
> 100644 MdeModulePkg/Include/Protocol/PlatformBootManager.h
>
> diff --git a/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> new file mode 100644
> index 0000000..ec32215
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/PlatformBootManager.h
> @@ -0,0 +1,82 @@
> +/** @file
> +
> + Copyright (c) 2019, NVIDIA CORPORATION. All rights reserved.
> +
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
> +#define __PLATFORM_BOOT_MANAGER_PROTOCOL_H__
> +
> +#include <Library/UefiBootManagerLib.h>
> +
> +//
> +// Platform Boot Manager Protocol GUID value // #define
> +EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL_GUID \
> + { \
> + 0xaa17add4, 0x756c, 0x460d, { 0x94, 0xb8, 0x43, 0x88, 0xd7,
> +0xfb, 0x3e,
> 0x59 } \
> + }
> +
> +//
> +// Protocol interface structure
> +//
> +typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL
> PLATFORM_BOOT_MANAGER_PROTOCOL;
For being consistent with other EDKII protocols like EDKII_PLATFORM_LOGO_PROTOCOL , how about we update it to the following?
typedef struct _EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL EDKII_PLATFORM_BOOT_MANAGER_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_MANAGER_PROTOCOL_REVISION
> 0x00000001
> +
> +//
> +// Function Prototypes
> +//
> +
> +/*
> + This function allows platform to refresh all boot options specific
> +to the
> platform. Within
> + this function, platform can make modifications to the auto
> + enumerated
> platform boot options
> + as well as NV boot options.
> +
> + @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[in const] BootOptionsCount The number of elements in
> BootOptions.
> +
> + @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
> REFRESH_ALL_BOOT_OPTIONS
> + and would be freed by caller after consuming it.
> +
> + @param[out] UpdatedBootOptionsCount The number of elements in
> UpdatedBootOptions.
> +
> +
> + @retval EFI_SUCCESS Platform refresh 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 *REFRESH_ALL_BOOT_OPTIONS) (
> + IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions,
> + IN CONST UINTN BootOptionsCount,
> + OUT EFI_BOOT_MANAGER_LOAD_OPTION **UpdatedBootOptions,
> + OUT UINTN *UpdatedBootOptionsCount
> + );
> +
> +struct _PLATFORM_BOOT_MANAGER_PROTOCOL {
> + UINT64 Revision;
> + REFRESH_ALL_BOOT_OPTIONS RefreshAllBootOptions; };
> +
> +extern EFI_GUID gEdkiiPlatformBootManagerProtocolGuid;
> +
> +#endif /* __PLATFORM_BOOT_MANAGER_PROTOCOL_H__ */
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 760d764..8b9a76d 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_MANAGER_PROTOCOL *PlatformBootManager;
>
> //
> // Optionally refresh the legacy boot option @@ -2273,7 +2277,6 @@
> EfiBootManagerRefreshAllBootOption (
> }
>
> BootOptions = BmEnumerateBootOptions (&BootOptionCount);
> - NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount,
> LoadOptionTypeBoot);
>
> //
> // Mark the boot option as added by BDS by setting OptionalData to
> a special GUID @@ -2284,6 +2287,30 @@
> EfiBootManagerRefreshAllBootOption (
> }
>
> //
> + // Locate Platform Boot Options Protocol // Status =
> + gBS->LocateProtocol (&gEdkiiPlatformBootManagerProtocolGuid,
> + NULL,
> + (VOID **)&PlatformBootManager); if
> + (!EFI_ERROR (Status)) {
> + //
> + // If found, call platform specific refresh to all auto enumerated and NV
> + // boot options.
> + //
> + Status = PlatformBootManager->RefreshAllBootOptions ((CONST
> EFI_BOOT_MANAGER_LOAD_OPTION *)BootOptions,
> + (CONST UINTN)BootOptionCount,
> + &UpdatedBootOptions,
> + &UpdatedBootOptionCount);
> + if (!EFI_ERROR (Status)) {
> + EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> + BootOptions = UpdatedBootOptions;
> + BootOptionCount = UpdatedBootOptionCount;
> + }
> + }
> +
> + NvBootOptions = EfiBootManagerGetLoadOptions (&NvBootOptionCount,
> LoadOptionTypeBoot);
> +
> + //
> // 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..ac866ac 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/PlatformBootManager.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..cf59086 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
> + gEdkiiPlatformBootManagerProtocolGuid ## SOMETIMES_CONSUMES
>
> [Pcd]
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC
> hange ## SOMETIMES_CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index 41b9e70..cc238e9 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/PlatformBootManager.h
> + gEdkiiPlatformBootManagerProtocolGuid = { 0xaa17add4, 0x756c,
> + 0x460d,
> { 0x94, 0xb8, 0x43, 0x88, 0xd7, 0xfb, 0x3e, 0x59 } }
> +
> #
> # [Error.gEfiMdeModulePkgTokenSpaceGuid]
> # 0x80000001 | Invalid value provided.
> --
> 2.7.4
-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-18 19:10 [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol Ashish Singhal
2019-12-19 1:59 ` Ni, Ray
@ 2019-12-23 1:46 ` Ni, Ray
2019-12-23 2:08 ` Ashish Singhal
1 sibling, 1 reply; 12+ messages in thread
From: Ni, Ray @ 2019-12-23 1:46 UTC (permalink / raw)
To: Ashish Singhal, devel@edk2.groups.io, Wang, Jian J, Wu, Hao A,
Gao, Zhichao
+ typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL PLATFORM_BOOT_MANAGER_PROTOCOL;
Ashish,
Please add EDKII_ prefix to the protocol structure as well: EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL.
I see you already added the prefix to the GUID name.
Thanks,
Ray
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-23 1:46 ` Ni, Ray
@ 2019-12-23 2:08 ` Ashish Singhal
2019-12-23 2:27 ` Ashish Singhal
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Singhal @ 2019-12-23 2:08 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io, Wang, Jian J, Wu, Hao A,
Gao, Zhichao
Done. Have submitted patch 5 for consideration.
Thanks
Ashish
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Sunday, December 22, 2019 6:47 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 v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
External email: Use caution opening links or attachments
+ typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL PLATFORM_BOOT_MANAGER_PROTOCOL;
Ashish,
Please add EDKII_ prefix to the protocol structure as well: EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL.
I see you already added the prefix to the GUID name.
Thanks,
Ray
-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-23 2:08 ` Ashish Singhal
@ 2019-12-23 2:27 ` Ashish Singhal
0 siblings, 0 replies; 12+ messages in thread
From: Ashish Singhal @ 2019-12-23 2:27 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io, Wang, Jian J, Wu, Hao A,
Gao, Zhichao
Please ignore patch 5. I have sent patch 6 for consideration.
Thanks
Ashish
-----Original Message-----
From: Ashish Singhal
Sent: Sunday, December 22, 2019 7:09 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 v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
Done. Have submitted patch 5 for consideration.
Thanks
Ashish
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Sunday, December 22, 2019 6:47 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 v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
External email: Use caution opening links or attachments
+ typedef struct _PLATFORM_BOOT_MANAGER_PROTOCOL PLATFORM_BOOT_MANAGER_PROTOCOL;
Ashish,
Please add EDKII_ prefix to the protocol structure as well: EDKII_PLATFORM_BOOT_MANAGER_PROTOCOL.
I see you already added the prefix to the GUID name.
Thanks,
Ray
-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-20 11:28 ` Wang, Sunny (HPS SW)
@ 2019-12-24 2:40 ` Ni, Ray
2019-12-24 4:37 ` Wang, Sunny (HPS SW)
0 siblings, 1 reply; 12+ messages in thread
From: Ni, Ray @ 2019-12-24 2:40 UTC (permalink / raw)
To: Wang, Sunny (HPS SW), devel@edk2.groups.io,
ashishsingha@nvidia.com, Wang, Jian J, Wu, Hao A, Gao, Zhichao,
Kinney, Michael D, 'Andrew Fish (afish@apple.com)'
Cc: Spottswood, Jason
> -----Original Message-----
> From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Sent: Friday, December 20, 2019 7:29 PM
> To: devel@edk2.groups.io; ashishsingha@nvidia.com; Ni, Ray
> <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)'
> <afish@apple.com>
> Cc: Spottswood, Jason <jason.spottswood@hpe.com>; Wang, Sunny (HPS
> SW) <sunnywang@hpe.com>
> Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform
> Boot Manager Protocol
>
> Good point. The way you used is more robust. It can cover a mistake in
> function's error handling. Thanks for clarifying this, Ashish.
>
> In addition, the other naming suggestion just comes to mind. How about we
> rename the function to a more generic one (based on location) like
> AfterEnumerateBootOptions or a more specific one like
> RefreshEnumeratedBootOptions? In the future, we may add the other hook
> function in the EfiBootManagerRefreshAllBootOption to deal with the boot
> options that are not created by BmEnumerateBootOptions. In this case (two
> hook functions in EfiBootManagerRefreshAllBootOption), the original
> function name "RefreshAllBootOptions" may cause some confusion.
Sunny,
What else feasibility do you think platform may require in future but this
RefreshAllBootOptions cannot support?
Thanks,
Ray
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-24 2:40 ` Ni, Ray
@ 2019-12-24 4:37 ` Wang, Sunny (HPS SW)
2019-12-24 4:48 ` Ashish Singhal
0 siblings, 1 reply; 12+ messages in thread
From: Wang, Sunny (HPS SW) @ 2019-12-24 4:37 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io, ashishsingha@nvidia.com,
Wang, Jian J, Wu, Hao A, Gao, Zhichao, Kinney, Michael D,
'Andrew Fish (afish@apple.com)'
Cc: Spottswood, Jason, Wang, Sunny (HPS SW)
Thanks for checking this, Ray.
Platform may want to:
1. Refresh the static boot options (that are not created by BmEnumerateBootOptions) without a reboot.
2. Update some other static-informational data or configuration data right after calling EfiBootManagerRefreshAllBootOption.
3. Always skip calling EfiBootManagerRefreshAllBootOption for the cases like BOOT_ASSUMING_NO_CONFIGURATION_CHANGES.
I know these actions can be done by adding code to other places, but using hooks in EfiBootManagerRefreshAllBootOption would be an easier solution for the platform. We won't need to take care of all the EfiBootManagerRefreshAllBootOption callers. Therefore, If we don't have a concern about adding more hooks and want to give the platform more flexibility, we could add two more hooks (1 and 3) in the future to have three hooks as below:
1. BeginOfRefreshAllBootOptions
2. RefreshAllBootOptions or RefreshEnumeratedBootOptions
3. EndOfRefreshAllBootOption
By the way, the current change looks good enough to me. In case Ashish or others are in urgent need of this code change, we can discuss my comments later in a separated email.
Regards,
Sunny Wang
-----Original Message-----
From: Ni, Ray [mailto:ray.ni@intel.com]
Sent: Tuesday, December 24, 2019 10:40 AM
To: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; devel@edk2.groups.io; ashishsingha@nvidia.com; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Spottswood, Jason <jason.spottswood@hpe.com>
Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
> -----Original Message-----
> From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Sent: Friday, December 20, 2019 7:29 PM
> To: devel@edk2.groups.io; ashishsingha@nvidia.com; Ni, Ray
> <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)'
> <afish@apple.com>
> Cc: Spottswood, Jason <jason.spottswood@hpe.com>; Wang, Sunny (HPS
> SW) <sunnywang@hpe.com>
> Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform
> Boot Manager Protocol
>
> Good point. The way you used is more robust. It can cover a mistake in
> function's error handling. Thanks for clarifying this, Ashish.
>
> In addition, the other naming suggestion just comes to mind. How about
> we rename the function to a more generic one (based on location) like
> AfterEnumerateBootOptions or a more specific one like
> RefreshEnumeratedBootOptions? In the future, we may add the other hook
> function in the EfiBootManagerRefreshAllBootOption to deal with the
> boot options that are not created by BmEnumerateBootOptions. In this
> case (two hook functions in EfiBootManagerRefreshAllBootOption), the
> original function name "RefreshAllBootOptions" may cause some confusion.
Sunny,
What else feasibility do you think platform may require in future but this RefreshAllBootOptions cannot support?
Thanks,
Ray
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-24 4:37 ` Wang, Sunny (HPS SW)
@ 2019-12-24 4:48 ` Ashish Singhal
2020-02-06 9:08 ` Wang, Sunny (HPS SW)
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Singhal @ 2019-12-24 4:48 UTC (permalink / raw)
To: Wang, Sunny (HPS SW), Ni, Ray, devel@edk2.groups.io, Wang, Jian J,
Wu, Hao A, Gao, Zhichao, Kinney, Michael D,
'Andrew Fish (afish@apple.com)'
Cc: Spottswood, Jason
Hi Sunny,
For the 3 use cases you suggested, please let me know if you think I am wrong.
1. RefreshAllBootOptions can refresh boot options which are auto created by BmEnumerateBootOptions as well as NV boot options in the variable store. Platform implementation of RefreshAllBootOptions can have calls to platform specific other libraries/drivers that create more boot options.
2. In EfiBootManagerRefreshAllBootOption, BmEnumerateBootOptions is the only function that populates boot options and then validates/invalidates them as well as NV boot options. RefreshAllBootOptions can modify static-informational data or configuration data from the boot options created by BmEnumerateBootOptions as well as in NV store.
3. Solution for third use case can be derived by using a PCD which can be defaulted to tell code to call EfiBootManagerRefreshAllBootOption every time but can be overridden by platform to not call it from anywhere except BDS.
Thanks
Ashish
-----Original Message-----
From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Sent: Monday, December 23, 2019 9:38 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Spottswood, Jason <jason.spottswood@hpe.com>; Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
External email: Use caution opening links or attachments
Thanks for checking this, Ray.
Platform may want to:
1. Refresh the static boot options (that are not created by BmEnumerateBootOptions) without a reboot.
2. Update some other static-informational data or configuration data right after calling EfiBootManagerRefreshAllBootOption.
3. Always skip calling EfiBootManagerRefreshAllBootOption for the cases like BOOT_ASSUMING_NO_CONFIGURATION_CHANGES.
I know these actions can be done by adding code to other places, but using hooks in EfiBootManagerRefreshAllBootOption would be an easier solution for the platform. We won't need to take care of all the EfiBootManagerRefreshAllBootOption callers. Therefore, If we don't have a concern about adding more hooks and want to give the platform more flexibility, we could add two more hooks (1 and 3) in the future to have three hooks as below:
1. BeginOfRefreshAllBootOptions
2. RefreshAllBootOptions or RefreshEnumeratedBootOptions
3. EndOfRefreshAllBootOption
By the way, the current change looks good enough to me. In case Ashish or others are in urgent need of this code change, we can discuss my comments later in a separated email.
Regards,
Sunny Wang
-----Original Message-----
From: Ni, Ray [mailto:ray.ni@intel.com]
Sent: Tuesday, December 24, 2019 10:40 AM
To: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; devel@edk2.groups.io; ashishsingha@nvidia.com; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Spottswood, Jason <jason.spottswood@hpe.com>
Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
> -----Original Message-----
> From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Sent: Friday, December 20, 2019 7:29 PM
> To: devel@edk2.groups.io; ashishsingha@nvidia.com; Ni, Ray
> <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)'
> <afish@apple.com>
> Cc: Spottswood, Jason <jason.spottswood@hpe.com>; Wang, Sunny (HPS
> SW) <sunnywang@hpe.com>
> Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform
> Boot Manager Protocol
>
> Good point. The way you used is more robust. It can cover a mistake in
> function's error handling. Thanks for clarifying this, Ashish.
>
> In addition, the other naming suggestion just comes to mind. How about
> we rename the function to a more generic one (based on location) like
> AfterEnumerateBootOptions or a more specific one like
> RefreshEnumeratedBootOptions? In the future, we may add the other hook
> function in the EfiBootManagerRefreshAllBootOption to deal with the
> boot options that are not created by BmEnumerateBootOptions. In this
> case (two hook functions in EfiBootManagerRefreshAllBootOption), the
> original function name "RefreshAllBootOptions" may cause some confusion.
Sunny,
What else feasibility do you think platform may require in future but this RefreshAllBootOptions cannot support?
Thanks,
Ray
-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
2019-12-24 4:48 ` Ashish Singhal
@ 2020-02-06 9:08 ` Wang, Sunny (HPS SW)
0 siblings, 0 replies; 12+ messages in thread
From: Wang, Sunny (HPS SW) @ 2020-02-06 9:08 UTC (permalink / raw)
To: Ashish Singhal, Ni, Ray, devel@edk2.groups.io, Wang, Jian J,
Wu, Hao A, Gao, Zhichao, Kinney, Michael D,
'Andrew Fish (afish@apple.com)'
Cc: Spottswood, Jason, Wang, Sunny (HPS SW)
Hi Ashish,
I was busy with some other stuff and forgot to reply to you. Sorry about that.
I was just thinking about adding more hooks would be easier for us to add platform code. However, it looks like your preference is to have only one hook. I'm also fine with this.
Moreover, your solutions look workable to me. I will give them a try. Thanks!
Regards,
Sunny Wang
-----Original Message-----
From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
Sent: Tuesday, December 24, 2019 12:48 PM
To: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; 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>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Spottswood, Jason <jason.spottswood@hpe.com>
Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
Hi Sunny,
For the 3 use cases you suggested, please let me know if you think I am wrong.
1. RefreshAllBootOptions can refresh boot options which are auto created by BmEnumerateBootOptions as well as NV boot options in the variable store. Platform implementation of RefreshAllBootOptions can have calls to platform specific other libraries/drivers that create more boot options.
2. In EfiBootManagerRefreshAllBootOption, BmEnumerateBootOptions is the only function that populates boot options and then validates/invalidates them as well as NV boot options. RefreshAllBootOptions can modify static-informational data or configuration data from the boot options created by BmEnumerateBootOptions as well as in NV store.
3. Solution for third use case can be derived by using a PCD which can be defaulted to tell code to call EfiBootManagerRefreshAllBootOption every time but can be overridden by platform to not call it from anywhere except BDS.
Thanks
Ashish
-----Original Message-----
From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Sent: Monday, December 23, 2019 9:38 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Spottswood, Jason <jason.spottswood@hpe.com>; Wang, Sunny (HPS SW) <sunnywang@hpe.com>
Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
External email: Use caution opening links or attachments
Thanks for checking this, Ray.
Platform may want to:
1. Refresh the static boot options (that are not created by BmEnumerateBootOptions) without a reboot.
2. Update some other static-informational data or configuration data right after calling EfiBootManagerRefreshAllBootOption.
3. Always skip calling EfiBootManagerRefreshAllBootOption for the cases like BOOT_ASSUMING_NO_CONFIGURATION_CHANGES.
I know these actions can be done by adding code to other places, but using hooks in EfiBootManagerRefreshAllBootOption would be an easier solution for the platform. We won't need to take care of all the EfiBootManagerRefreshAllBootOption callers. Therefore, If we don't have a concern about adding more hooks and want to give the platform more flexibility, we could add two more hooks (1 and 3) in the future to have three hooks as below:
1. BeginOfRefreshAllBootOptions
2. RefreshAllBootOptions or RefreshEnumeratedBootOptions
3. EndOfRefreshAllBootOption
By the way, the current change looks good enough to me. In case Ashish or others are in urgent need of this code change, we can discuss my comments later in a separated email.
Regards,
Sunny Wang
-----Original Message-----
From: Ni, Ray [mailto:ray.ni@intel.com]
Sent: Tuesday, December 24, 2019 10:40 AM
To: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; devel@edk2.groups.io; ashishsingha@nvidia.com; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)' <afish@apple.com>
Cc: Spottswood, Jason <jason.spottswood@hpe.com>
Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol
> -----Original Message-----
> From: Wang, Sunny (HPS SW) <sunnywang@hpe.com>
> Sent: Friday, December 20, 2019 7:29 PM
> To: devel@edk2.groups.io; ashishsingha@nvidia.com; Ni, Ray
> <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; 'Andrew Fish (afish@apple.com)'
> <afish@apple.com>
> Cc: Spottswood, Jason <jason.spottswood@hpe.com>; Wang, Sunny (HPS
> SW) <sunnywang@hpe.com>
> Subject: RE: [edk2-devel] [PATCH v4] MdeModulePkg: Add EDK2 Platform
> Boot Manager Protocol
>
> Good point. The way you used is more robust. It can cover a mistake in
> function's error handling. Thanks for clarifying this, Ashish.
>
> In addition, the other naming suggestion just comes to mind. How about
> we rename the function to a more generic one (based on location) like
> AfterEnumerateBootOptions or a more specific one like
> RefreshEnumeratedBootOptions? In the future, we may add the other hook
> function in the EfiBootManagerRefreshAllBootOption to deal with the
> boot options that are not created by BmEnumerateBootOptions. In this
> case (two hook functions in EfiBootManagerRefreshAllBootOption), the
> original function name "RefreshAllBootOptions" may cause some confusion.
Sunny,
What else feasibility do you think platform may require in future but this RefreshAllBootOptions cannot support?
Thanks,
Ray
-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-02-06 9:08 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-18 19:10 [PATCH v4] MdeModulePkg: Add EDK2 Platform Boot Manager Protocol Ashish Singhal
2019-12-19 1:59 ` Ni, Ray
2019-12-19 14:22 ` [edk2-devel] " Wang, Sunny (HPS SW)
2019-12-19 16:38 ` Ashish Singhal
2019-12-20 11:28 ` Wang, Sunny (HPS SW)
2019-12-24 2:40 ` Ni, Ray
2019-12-24 4:37 ` Wang, Sunny (HPS SW)
2019-12-24 4:48 ` Ashish Singhal
2020-02-06 9:08 ` Wang, Sunny (HPS SW)
2019-12-23 1:46 ` Ni, Ray
2019-12-23 2:08 ` Ashish Singhal
2019-12-23 2:27 ` Ashish Singhal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox