public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
@ 2022-05-16 23:01 Ashish Singhal
  2022-05-17 14:11 ` [edk2-devel] " Ni, Ray
  0 siblings, 1 reply; 8+ messages in thread
From: Ashish Singhal @ 2022-05-16 23:01 UTC (permalink / raw)
  To: devel, jian.j.wang, gaoliming, zhichao.gao, ray.ni; +Cc: Ashish Singhal

Add a new PCD to be able to configure whether newly detected boot options
are to be added at the beginning of the current boot options list or at
the end.

Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c            | 6 +++++-
 .../Library/UefiBootManagerLib/UefiBootManagerLib.inf       | 1 +
 MdeModulePkg/MdeModulePkg.dec                               | 5 +++++
 MdeModulePkg/MdeModulePkg.uni                               | 4 ++++
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 962892d38f..8a46100c2a 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2435,7 +2435,11 @@ EfiBootManagerRefreshAllBootOption (
   //
   for (Index = 0; Index < BootOptionCount; Index++) {
     if (EfiBootManagerFindLoadOption (&BootOptions[Index], NvBootOptions, NvBootOptionCount) == -1) {
-      EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
+      if (PcdGetBool (PcdNewBootOptionAtStart)) {
+        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], 0);
+      } else {
+        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
+      }
       //
       // Try best to add the boot options so continue upon failure.
       //
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index fe05d5f1cc..46f41a7c63 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -119,3 +119,4 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart                    ## CONSUMES
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index cf79292ec8..9d696f117b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -2146,6 +2146,11 @@
   # @Prompt GHCB Pool Size
   gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0|UINT64|0x00030008
 
+  ## This dynamic PCD holds the flag to tell UEFI boot manager whether to add newly detected devices at
+  #  the end, or at the start of the boot option.
+  # @Prompt Add new devices in boot options at start
+  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart|FALSE|BOOLEAN|0x00030009
+
 [PcdsDynamicEx]
   ## This dynamic PCD enables the default variable setting.
   #  Its value is the default store ID value. The default value is zero as Standard default.
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index b070f15ff2..8e68db1c25 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -1325,6 +1325,10 @@
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_HELP #language en-US "Used with SEV-ES support to identify an address range that is not to be encrypted."
 
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_PROMPT #language en-US "Add new devices in boot options at start"
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_HELP #language en-US "Used by UEFI boot manager to decide whether to place newly detcted devices at start of the list or end."
+
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_PROMPT #language en-US "Guest-Hypervisor Communication Block (GHCB) Pool Base Size"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_HELP #language en-US "Used with SEV-ES support to identify the size of the address range that is not to be encrypted."
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
  2022-05-16 23:01 [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options Ashish Singhal
@ 2022-05-17 14:11 ` Ni, Ray
  2022-05-17 16:08   ` Ashish Singhal
  0 siblings, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2022-05-17 14:11 UTC (permalink / raw)
  To: devel@edk2.groups.io, ashishsingha@nvidia.com, Wang, Jian J,
	Gao, Liming, Gao, Zhichao

Please use the EfiBootManagerSortLoadOptionVariable() to sort the boot options from PlatformBootManagerLib.


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ashish Singhal via groups.io
> Sent: Tuesday, May 17, 2022 7:02 AM
> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao
> <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
> 
> Add a new PCD to be able to configure whether newly detected boot options
> are to be added at the beginning of the current boot options list or at
> the end.
> 
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c            | 6 +++++-
>  .../Library/UefiBootManagerLib/UefiBootManagerLib.inf       | 1 +
>  MdeModulePkg/MdeModulePkg.dec                               | 5 +++++
>  MdeModulePkg/MdeModulePkg.uni                               | 4 ++++
>  4 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 962892d38f..8a46100c2a 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -2435,7 +2435,11 @@ EfiBootManagerRefreshAllBootOption (
>    //
>    for (Index = 0; Index < BootOptionCount; Index++) {
>      if (EfiBootManagerFindLoadOption (&BootOptions[Index], NvBootOptions, NvBootOptionCount) == -1) {
> -      EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      if (PcdGetBool (PcdNewBootOptionAtStart)) {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], 0);
> +      } else {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      }
>        //
>        // Try best to add the boot options so continue upon failure.
>        //
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index fe05d5f1cc..46f41a7c63 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -119,3 +119,4 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart                    ## CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index cf79292ec8..9d696f117b 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2146,6 +2146,11 @@
>    # @Prompt GHCB Pool Size
>    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0|UINT64|0x00030008
> 
> +  ## This dynamic PCD holds the flag to tell UEFI boot manager whether to add newly detected devices at
> +  #  the end, or at the start of the boot option.
> +  # @Prompt Add new devices in boot options at start
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart|FALSE|BOOLEAN|0x00030009
> +
>  [PcdsDynamicEx]
>    ## This dynamic PCD enables the default variable setting.
>    #  Its value is the default store ID value. The default value is zero as Standard default.
> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
> index b070f15ff2..8e68db1c25 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1325,6 +1325,10 @@
> 
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_HELP #language en-US "Used with SEV-ES support to identify
> an address range that is not to be encrypted."
> 
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_PROMPT #language en-US "Add new devices in
> boot options at start"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_HELP #language en-US "Used by UEFI boot
> manager to decide whether to place newly detcted devices at start of the list or end."
> +
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_PROMPT #language en-US "Guest-Hypervisor Communication
> Block (GHCB) Pool Base Size"
> 
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_HELP #language en-US "Used with SEV-ES support to identify the
> size of the address range that is not to be encrypted."
> --
> 2.17.1
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
  2022-05-17 14:11 ` [edk2-devel] " Ni, Ray
@ 2022-05-17 16:08   ` Ashish Singhal
  2022-05-19 15:50     ` Ashish Singhal
  2022-05-26 13:15     ` Ni, Ray
  0 siblings, 2 replies; 8+ messages in thread
From: Ashish Singhal @ 2022-05-17 16:08 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Wang, Jian J, Gao, Liming,
	Gao, Zhichao

[-- Attachment #1: Type: text/plain, Size: 5582 bytes --]

Ray,

Won't that mean changing UEFI UI and boot maintenance manager applications as well to make this call? If we do not do that, the applications would not reflect the sorted boot order automatically.

Considering this, the change I have is pretty small and takes care of all scenarios. Please let me know what you think.

Thanks
Ashish
________________________________
From: Ni, Ray <ray.ni@intel.com>
Sent: Tuesday, May 17, 2022 8:11 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options

External email: Use caution opening links or attachments


Please use the EfiBootManagerSortLoadOptionVariable() to sort the boot options from PlatformBootManagerLib.


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ashish Singhal via groups.io
> Sent: Tuesday, May 17, 2022 7:02 AM
> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao
> <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
>
> Add a new PCD to be able to configure whether newly detected boot options
> are to be added at the beginning of the current boot options list or at
> the end.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c            | 6 +++++-
>  .../Library/UefiBootManagerLib/UefiBootManagerLib.inf       | 1 +
>  MdeModulePkg/MdeModulePkg.dec                               | 5 +++++
>  MdeModulePkg/MdeModulePkg.uni                               | 4 ++++
>  4 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 962892d38f..8a46100c2a 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -2435,7 +2435,11 @@ EfiBootManagerRefreshAllBootOption (
>    //
>    for (Index = 0; Index < BootOptionCount; Index++) {
>      if (EfiBootManagerFindLoadOption (&BootOptions[Index], NvBootOptions, NvBootOptionCount) == -1) {
> -      EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      if (PcdGetBool (PcdNewBootOptionAtStart)) {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], 0);
> +      } else {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      }
>        //
>        // Try best to add the boot options so continue upon failure.
>        //
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index fe05d5f1cc..46f41a7c63 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -119,3 +119,4 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart                    ## CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index cf79292ec8..9d696f117b 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2146,6 +2146,11 @@
>    # @Prompt GHCB Pool Size
>    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0|UINT64|0x00030008
>
> +  ## This dynamic PCD holds the flag to tell UEFI boot manager whether to add newly detected devices at
> +  #  the end, or at the start of the boot option.
> +  # @Prompt Add new devices in boot options at start
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart|FALSE|BOOLEAN|0x00030009
> +
>  [PcdsDynamicEx]
>    ## This dynamic PCD enables the default variable setting.
>    #  Its value is the default store ID value. The default value is zero as Standard default.
> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
> index b070f15ff2..8e68db1c25 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1325,6 +1325,10 @@
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_HELP #language en-US "Used with SEV-ES support to identify
> an address range that is not to be encrypted."
>
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_PROMPT #language en-US "Add new devices in
> boot options at start"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_HELP #language en-US "Used by UEFI boot
> manager to decide whether to place newly detcted devices at start of the list or end."
> +
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_PROMPT #language en-US "Guest-Hypervisor Communication
> Block (GHCB) Pool Base Size"
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_HELP #language en-US "Used with SEV-ES support to identify the
> size of the address range that is not to be encrypted."
> --
> 2.17.1
>
>
>
> 
>


[-- Attachment #2: Type: text/html, Size: 9166 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
  2022-05-17 16:08   ` Ashish Singhal
@ 2022-05-19 15:50     ` Ashish Singhal
  2022-05-26 13:15     ` Ni, Ray
  1 sibling, 0 replies; 8+ messages in thread
From: Ashish Singhal @ 2022-05-19 15:50 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Wang, Jian J, Gao, Liming,
	Gao, Zhichao

[-- Attachment #1: Type: text/plain, Size: 6042 bytes --]

Ray,

Any update on this?

Thanks
Ashish
________________________________
From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Tuesday, May 17, 2022 10:08 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options

Ray,

Won't that mean changing UEFI UI and boot maintenance manager applications as well to make this call? If we do not do that, the applications would not reflect the sorted boot order automatically.

Considering this, the change I have is pretty small and takes care of all scenarios. Please let me know what you think.

Thanks
Ashish
________________________________
From: Ni, Ray <ray.ni@intel.com>
Sent: Tuesday, May 17, 2022 8:11 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ashish Singhal <ashishsingha@nvidia.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options

External email: Use caution opening links or attachments


Please use the EfiBootManagerSortLoadOptionVariable() to sort the boot options from PlatformBootManagerLib.


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ashish Singhal via groups.io
> Sent: Tuesday, May 17, 2022 7:02 AM
> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao
> <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>
> Cc: Ashish Singhal <ashishsingha@nvidia.com>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
>
> Add a new PCD to be able to configure whether newly detected boot options
> are to be added at the beginning of the current boot options list or at
> the end.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c            | 6 +++++-
>  .../Library/UefiBootManagerLib/UefiBootManagerLib.inf       | 1 +
>  MdeModulePkg/MdeModulePkg.dec                               | 5 +++++
>  MdeModulePkg/MdeModulePkg.uni                               | 4 ++++
>  4 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 962892d38f..8a46100c2a 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -2435,7 +2435,11 @@ EfiBootManagerRefreshAllBootOption (
>    //
>    for (Index = 0; Index < BootOptionCount; Index++) {
>      if (EfiBootManagerFindLoadOption (&BootOptions[Index], NvBootOptions, NvBootOptionCount) == -1) {
> -      EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      if (PcdGetBool (PcdNewBootOptionAtStart)) {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], 0);
> +      } else {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      }
>        //
>        // Try best to add the boot options so continue upon failure.
>        //
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index fe05d5f1cc..46f41a7c63 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -119,3 +119,4 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart                    ## CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index cf79292ec8..9d696f117b 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2146,6 +2146,11 @@
>    # @Prompt GHCB Pool Size
>    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0|UINT64|0x00030008
>
> +  ## This dynamic PCD holds the flag to tell UEFI boot manager whether to add newly detected devices at
> +  #  the end, or at the start of the boot option.
> +  # @Prompt Add new devices in boot options at start
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart|FALSE|BOOLEAN|0x00030009
> +
>  [PcdsDynamicEx]
>    ## This dynamic PCD enables the default variable setting.
>    #  Its value is the default store ID value. The default value is zero as Standard default.
> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
> index b070f15ff2..8e68db1c25 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1325,6 +1325,10 @@
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_HELP #language en-US "Used with SEV-ES support to identify
> an address range that is not to be encrypted."
>
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_PROMPT #language en-US "Add new devices in
> boot options at start"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_HELP #language en-US "Used by UEFI boot
> manager to decide whether to place newly detcted devices at start of the list or end."
> +
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_PROMPT #language en-US "Guest-Hypervisor Communication
> Block (GHCB) Pool Base Size"
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_HELP #language en-US "Used with SEV-ES support to identify the
> size of the address range that is not to be encrypted."
> --
> 2.17.1
>
>
>
> 
>


[-- Attachment #2: Type: text/html, Size: 10641 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
  2022-05-17 16:08   ` Ashish Singhal
  2022-05-19 15:50     ` Ashish Singhal
@ 2022-05-26 13:15     ` Ni, Ray
  2022-05-26 13:16       ` Ashish Singhal
  1 sibling, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2022-05-26 13:15 UTC (permalink / raw)
  To: Ashish Singhal, devel@edk2.groups.io, Wang, Jian J, Gao, Liming,
	Gao, Zhichao

[-- Attachment #1: Type: text/plain, Size: 6603 bytes --]

  1.  I don't like PCD. It's like a out-of-band control of library API.
  2.  The PCD doesn't take care of all scenarios. Someone may like to put new options just before the last one.

From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Wednesday, May 18, 2022 12:09 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options

Ray,

Won't that mean changing UEFI UI and boot maintenance manager applications as well to make this call? If we do not do that, the applications would not reflect the sorted boot order automatically.

Considering this, the change I have is pretty small and takes care of all scenarios. Please let me know what you think.

Thanks
Ashish
________________________________
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Tuesday, May 17, 2022 8:11 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options

External email: Use caution opening links or attachments


Please use the EfiBootManagerSortLoadOptionVariable() to sort the boot options from PlatformBootManagerLib.


> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ashish Singhal via groups.io
> Sent: Tuesday, May 17, 2022 7:02 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao
> <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
>
> Add a new PCD to be able to configure whether newly detected boot options
> are to be added at the beginning of the current boot options list or at
> the end.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c            | 6 +++++-
>  .../Library/UefiBootManagerLib/UefiBootManagerLib.inf       | 1 +
>  MdeModulePkg/MdeModulePkg.dec                               | 5 +++++
>  MdeModulePkg/MdeModulePkg.uni                               | 4 ++++
>  4 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 962892d38f..8a46100c2a 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -2435,7 +2435,11 @@ EfiBootManagerRefreshAllBootOption (
>    //
>    for (Index = 0; Index < BootOptionCount; Index++) {
>      if (EfiBootManagerFindLoadOption (&BootOptions[Index], NvBootOptions, NvBootOptionCount) == -1) {
> -      EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      if (PcdGetBool (PcdNewBootOptionAtStart)) {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], 0);
> +      } else {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      }
>        //
>        // Try best to add the boot options so continue upon failure.
>        //
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index fe05d5f1cc..46f41a7c63 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -119,3 +119,4 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart                    ## CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index cf79292ec8..9d696f117b 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2146,6 +2146,11 @@
>    # @Prompt GHCB Pool Size
>    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0|UINT64|0x00030008
>
> +  ## This dynamic PCD holds the flag to tell UEFI boot manager whether to add newly detected devices at
> +  #  the end, or at the start of the boot option.
> +  # @Prompt Add new devices in boot options at start
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart|FALSE|BOOLEAN|0x00030009
> +
>  [PcdsDynamicEx]
>    ## This dynamic PCD enables the default variable setting.
>    #  Its value is the default store ID value. The default value is zero as Standard default.
> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
> index b070f15ff2..8e68db1c25 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1325,6 +1325,10 @@
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_HELP #language en-US "Used with SEV-ES support to identify
> an address range that is not to be encrypted."
>
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_PROMPT #language en-US "Add new devices in
> boot options at start"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_HELP #language en-US "Used by UEFI boot
> manager to decide whether to place newly detcted devices at start of the list or end."
> +
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_PROMPT #language en-US "Guest-Hypervisor Communication
> Block (GHCB) Pool Base Size"
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_HELP #language en-US "Used with SEV-ES support to identify the
> size of the address range that is not to be encrypted."
> --
> 2.17.1
>
>
>
> 
>

[-- Attachment #2: Type: text/html, Size: 14219 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
  2022-05-26 13:15     ` Ni, Ray
@ 2022-05-26 13:16       ` Ashish Singhal
  2022-05-26 13:29         ` Ni, Ray
  0 siblings, 1 reply; 8+ messages in thread
From: Ashish Singhal @ 2022-05-26 13:16 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Wang, Jian J, Gao, Liming,
	Gao, Zhichao

[-- Attachment #1: Type: text/plain, Size: 7280 bytes --]

I understand that. What do you suggest then? Using EfiBootManagerSortLoadOptionVariable() is an option only if we update all code paths where we refresh the boot options.
________________________________
From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, May 26, 2022 7:15 AM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options

External email: Use caution opening links or attachments


  1.  I don’t like PCD. It’s like a out-of-band control of library API.
  2.  The PCD doesn’t take care of all scenarios. Someone may like to put new options just before the last one.



From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Wednesday, May 18, 2022 12:09 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options



Ray,



Won't that mean changing UEFI UI and boot maintenance manager applications as well to make this call? If we do not do that, the applications would not reflect the sorted boot order automatically.



Considering this, the change I have is pretty small and takes care of all scenarios. Please let me know what you think.



Thanks

Ashish

________________________________

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Tuesday, May 17, 2022 8:11 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options



External email: Use caution opening links or attachments


Please use the EfiBootManagerSortLoadOptionVariable() to sort the boot options from PlatformBootManagerLib.


> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ashish Singhal via groups.io
> Sent: Tuesday, May 17, 2022 7:02 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao
> <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
>
> Add a new PCD to be able to configure whether newly detected boot options
> are to be added at the beginning of the current boot options list or at
> the end.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c            | 6 +++++-
>  .../Library/UefiBootManagerLib/UefiBootManagerLib.inf       | 1 +
>  MdeModulePkg/MdeModulePkg.dec                               | 5 +++++
>  MdeModulePkg/MdeModulePkg.uni                               | 4 ++++
>  4 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 962892d38f..8a46100c2a 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -2435,7 +2435,11 @@ EfiBootManagerRefreshAllBootOption (
>    //
>    for (Index = 0; Index < BootOptionCount; Index++) {
>      if (EfiBootManagerFindLoadOption (&BootOptions[Index], NvBootOptions, NvBootOptionCount) == -1) {
> -      EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      if (PcdGetBool (PcdNewBootOptionAtStart)) {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], 0);
> +      } else {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      }
>        //
>        // Try best to add the boot options so continue upon failure.
>        //
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index fe05d5f1cc..46f41a7c63 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -119,3 +119,4 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart                    ## CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index cf79292ec8..9d696f117b 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2146,6 +2146,11 @@
>    # @Prompt GHCB Pool Size
>    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0|UINT64|0x00030008
>
> +  ## This dynamic PCD holds the flag to tell UEFI boot manager whether to add newly detected devices at
> +  #  the end, or at the start of the boot option.
> +  # @Prompt Add new devices in boot options at start
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart|FALSE|BOOLEAN|0x00030009
> +
>  [PcdsDynamicEx]
>    ## This dynamic PCD enables the default variable setting.
>    #  Its value is the default store ID value. The default value is zero as Standard default.
> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
> index b070f15ff2..8e68db1c25 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1325,6 +1325,10 @@
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_HELP #language en-US "Used with SEV-ES support to identify
> an address range that is not to be encrypted."
>
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_PROMPT #language en-US "Add new devices in
> boot options at start"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_HELP #language en-US "Used by UEFI boot
> manager to decide whether to place newly detcted devices at start of the list or end."
> +
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_PROMPT #language en-US "Guest-Hypervisor Communication
> Block (GHCB) Pool Base Size"
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_HELP #language en-US "Used with SEV-ES support to identify the
> size of the address range that is not to be encrypted."
> --
> 2.17.1
>
>
>
> 
>

[-- Attachment #2: Type: text/html, Size: 13028 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
  2022-05-26 13:16       ` Ashish Singhal
@ 2022-05-26 13:29         ` Ni, Ray
  2022-05-26 13:37           ` Ashish Singhal
  0 siblings, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2022-05-26 13:29 UTC (permalink / raw)
  To: Ashish Singhal, devel@edk2.groups.io, Wang, Jian J, Gao, Liming,
	Gao, Zhichao

[-- Attachment #1: Type: text/plain, Size: 8106 bytes --]

Yes. UefiBootManagerLib is a fundamental BDS library. Please change the caller.

From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Thursday, May 26, 2022 9:17 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options

I understand that. What do you suggest then? Using EfiBootManagerSortLoadOptionVariable() is an option only if we update all code paths where we refresh the boot options.
________________________________
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, May 26, 2022 7:15 AM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options

External email: Use caution opening links or attachments


  1.  I don't like PCD. It's like a out-of-band control of library API.
  2.  The PCD doesn't take care of all scenarios. Someone may like to put new options just before the last one.



From: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
Sent: Wednesday, May 18, 2022 12:09 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options



Ray,



Won't that mean changing UEFI UI and boot maintenance manager applications as well to make this call? If we do not do that, the applications would not reflect the sorted boot order automatically.



Considering this, the change I have is pretty small and takes care of all scenarios. Please let me know what you think.



Thanks

Ashish

________________________________

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Tuesday, May 17, 2022 8:11 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options



External email: Use caution opening links or attachments


Please use the EfiBootManagerSortLoadOptionVariable() to sort the boot options from PlatformBootManagerLib.


> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ashish Singhal via groups.io
> Sent: Tuesday, May 17, 2022 7:02 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao
> <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
>
> Add a new PCD to be able to configure whether newly detected boot options
> are to be added at the beginning of the current boot options list or at
> the end.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c            | 6 +++++-
>  .../Library/UefiBootManagerLib/UefiBootManagerLib.inf       | 1 +
>  MdeModulePkg/MdeModulePkg.dec                               | 5 +++++
>  MdeModulePkg/MdeModulePkg.uni                               | 4 ++++
>  4 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 962892d38f..8a46100c2a 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -2435,7 +2435,11 @@ EfiBootManagerRefreshAllBootOption (
>    //
>    for (Index = 0; Index < BootOptionCount; Index++) {
>      if (EfiBootManagerFindLoadOption (&BootOptions[Index], NvBootOptions, NvBootOptionCount) == -1) {
> -      EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      if (PcdGetBool (PcdNewBootOptionAtStart)) {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], 0);
> +      } else {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      }
>        //
>        // Try best to add the boot options so continue upon failure.
>        //
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index fe05d5f1cc..46f41a7c63 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -119,3 +119,4 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart                    ## CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index cf79292ec8..9d696f117b 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2146,6 +2146,11 @@
>    # @Prompt GHCB Pool Size
>    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0|UINT64|0x00030008
>
> +  ## This dynamic PCD holds the flag to tell UEFI boot manager whether to add newly detected devices at
> +  #  the end, or at the start of the boot option.
> +  # @Prompt Add new devices in boot options at start
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart|FALSE|BOOLEAN|0x00030009
> +
>  [PcdsDynamicEx]
>    ## This dynamic PCD enables the default variable setting.
>    #  Its value is the default store ID value. The default value is zero as Standard default.
> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
> index b070f15ff2..8e68db1c25 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1325,6 +1325,10 @@
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_HELP #language en-US "Used with SEV-ES support to identify
> an address range that is not to be encrypted."
>
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_PROMPT #language en-US "Add new devices in
> boot options at start"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_HELP #language en-US "Used by UEFI boot
> manager to decide whether to place newly detcted devices at start of the list or end."
> +
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_PROMPT #language en-US "Guest-Hypervisor Communication
> Block (GHCB) Pool Base Size"
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_HELP #language en-US "Used with SEV-ES support to identify the
> size of the address range that is not to be encrypted."
> --
> 2.17.1
>
>
>
> 
>

[-- Attachment #2: Type: text/html, Size: 16249 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
  2022-05-26 13:29         ` Ni, Ray
@ 2022-05-26 13:37           ` Ashish Singhal
  0 siblings, 0 replies; 8+ messages in thread
From: Ashish Singhal @ 2022-05-26 13:37 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io, Wang, Jian J, Gao, Liming,
	Gao, Zhichao

[-- Attachment #1: Type: text/plain, Size: 8902 bytes --]

That will not work either in all scenarios as the sort function will need to be a part of the caller. If the caller is an entity in edk2, then having a custom sort function cannot be achieved without a NULL library also being added to edk2 that can be custom implemented by the user.

Thanks
Ashish
________________________________
From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, May 26, 2022 7:29 AM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options

External email: Use caution opening links or attachments


Yes. UefiBootManagerLib is a fundamental BDS library. Please change the caller.



From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Thursday, May 26, 2022 9:17 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Gao, Zhichao <zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options



I understand that. What do you suggest then? Using EfiBootManagerSortLoadOptionVariable() is an option only if we update all code paths where we refresh the boot options.

________________________________

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, May 26, 2022 7:15 AM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options



External email: Use caution opening links or attachments



  1.  I don’t like PCD. It’s like a out-of-band control of library API.
  2.  The PCD doesn’t take care of all scenarios. Someone may like to put new options just before the last one.



From: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
Sent: Wednesday, May 18, 2022 12:09 AM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options



Ray,



Won't that mean changing UEFI UI and boot maintenance manager applications as well to make this call? If we do not do that, the applications would not reflect the sorted boot order automatically.



Considering this, the change I have is pretty small and takes care of all scenarios. Please let me know what you think.



Thanks

Ashish

________________________________

From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Tuesday, May 17, 2022 8:11 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options



External email: Use caution opening links or attachments


Please use the EfiBootManagerSortLoadOptionVariable() to sort the boot options from PlatformBootManagerLib.


> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ashish Singhal via groups.io
> Sent: Tuesday, May 17, 2022 7:02 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Gao, Liming <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Gao, Zhichao
> <zhichao.gao@intel.com<mailto:zhichao.gao@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Cc: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options
>
> Add a new PCD to be able to configure whether newly detected boot options
> are to be added at the beginning of the current boot options list or at
> the end.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c            | 6 +++++-
>  .../Library/UefiBootManagerLib/UefiBootManagerLib.inf       | 1 +
>  MdeModulePkg/MdeModulePkg.dec                               | 5 +++++
>  MdeModulePkg/MdeModulePkg.uni                               | 4 ++++
>  4 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 962892d38f..8a46100c2a 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -2435,7 +2435,11 @@ EfiBootManagerRefreshAllBootOption (
>    //
>    for (Index = 0; Index < BootOptionCount; Index++) {
>      if (EfiBootManagerFindLoadOption (&BootOptions[Index], NvBootOptions, NvBootOptionCount) == -1) {
> -      EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      if (PcdGetBool (PcdNewBootOptionAtStart)) {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], 0);
> +      } else {
> +        EfiBootManagerAddLoadOptionVariable (&BootOptions[Index], (UINTN)-1);
> +      }
>        //
>        // Try best to add the boot options so continue upon failure.
>        //
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> index fe05d5f1cc..46f41a7c63 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> @@ -119,3 +119,4 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile                     ## CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDriverHealthConfigureForm               ## SOMETIMES_CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxRepairCount                          ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart                    ## CONSUMES
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index cf79292ec8..9d696f117b 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -2146,6 +2146,11 @@
>    # @Prompt GHCB Pool Size
>    gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize|0|UINT64|0x00030008
>
> +  ## This dynamic PCD holds the flag to tell UEFI boot manager whether to add newly detected devices at
> +  #  the end, or at the start of the boot option.
> +  # @Prompt Add new devices in boot options at start
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNewBootOptionAtStart|FALSE|BOOLEAN|0x00030009
> +
>  [PcdsDynamicEx]
>    ## This dynamic PCD enables the default variable setting.
>    #  Its value is the default store ID value. The default value is zero as Standard default.
> diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
> index b070f15ff2..8e68db1c25 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -1325,6 +1325,10 @@
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbBase_HELP #language en-US "Used with SEV-ES support to identify
> an address range that is not to be encrypted."
>
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_PROMPT #language en-US "Add new devices in
> boot options at start"
> +
> +#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNewBootOptionAtStart_HELP #language en-US "Used by UEFI boot
> manager to decide whether to place newly detcted devices at start of the list or end."
> +
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_PROMPT #language en-US "Guest-Hypervisor Communication
> Block (GHCB) Pool Base Size"
>
>  #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdGhcbSize_HELP #language en-US "Used with SEV-ES support to identify the
> size of the address range that is not to be encrypted."
> --
> 2.17.1
>
>
>
> 
>

[-- Attachment #2: Type: text/html, Size: 16554 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-05-26 13:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-16 23:01 [PATCH] MdeModulePkg/UefiBootManagerLib: Configurable New Boot Options Ashish Singhal
2022-05-17 14:11 ` [edk2-devel] " Ni, Ray
2022-05-17 16:08   ` Ashish Singhal
2022-05-19 15:50     ` Ashish Singhal
2022-05-26 13:15     ` Ni, Ray
2022-05-26 13:16       ` Ashish Singhal
2022-05-26 13:29         ` Ni, Ray
2022-05-26 13:37           ` Ashish Singhal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox