[PATCH] MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder

[PATCH] MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder

[Li, Walon]

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3135

When Boot Menu does not exist in the BootOrder, BmRegisterBootManagerMenu
will create one into list. However, it should be put at the "end" of
BootOrder instead of "start" of BootOrder. Replace 0 by -1 to adjust
order of load options.

Signed-off-by: Walon Li <walon.li@hpe.com>
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index aff620ad52..26d1fb0ea0 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2505,7 +2505,7 @@ BmRegisterBootManagerMenu (
     EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
     );
 
-  return EfiBootManagerAddLoadOptionVariable (BootOption, 0);
+  return EfiBootManagerAddLoadOptionVariable (BootOption, (UINTN) -1));
 }
 
 /**
-- 
2.23.0.windows.1

回复: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder

[gaoliming]

Walon:
  Can you specify the detail reason why BootManagerMenu should be placed at
end of BootOrder?

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+71766+4905953+8761045@groups.io
> <bounce+27952+71766+4905953+8761045@groups.io> 代表 Li, Walon
> 发送时间: 2021年2月18日 11:26
> 收件人: devel@edk2.groups.io
> 抄送: walon.li@hpe.com; sunnywang@hpe.com; lersek@redhat.com;
> ray.ni@intel.com; hao.a.wu@intel.com
> 主题: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Put
> BootMenu at the end of BootOrder
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3135
> 
> When Boot Menu does not exist in the BootOrder,
> BmRegisterBootManagerMenu
> will create one into list. However, it should be put at the "end" of
> BootOrder instead of "start" of BootOrder. Replace 0 by -1 to adjust
> order of load options.
> 
> Signed-off-by: Walon Li <walon.li@hpe.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index aff620ad52..26d1fb0ea0 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -2505,7 +2505,7 @@ BmRegisterBootManagerMenu (
>      EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> 
>      );
> 
> 
> 
> -  return EfiBootManagerAddLoadOptionVariable (BootOption, 0);
> 
> +  return EfiBootManagerAddLoadOptionVariable (BootOption, (UINTN) -1));
> 
>  }
> 
> 
> 
>  /**
> 
> --
> 2.23.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71766): https://edk2.groups.io/g/devel/message/71766
> Mute This Topic: https://groups.io/mt/80721971/4905953
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
> 

Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder

[Li, Walon]

Hi Liming,

As edk2 design, any new boot options should be put at the end of BootOrder because these are NEW . That means system should "append" BootOrder instead of override original order.
For example, if system has three boot options currently - Boot0001, Boot0002, Boot0003 and then one new option - Boot0000 will be added. The order should become Boot0001,Boot0002,Boot0003,Boot0000. However, in this case, BootmanagerMenu doesn't follow this rule. We set "zero" priority so system would put BootManagerMenu boot option at start.
This case is a corner case because the symptom only be gotten when user delete BootManagerMenu on OS or EFI shell. But it's a possible case. For keeping behavior consistent, we should keep BootManagerMenu option behavior as same as others boot option.

Thanks
Walon

-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn> 
Sent: Friday, February 19, 2021 8:59 AM
To: devel@edk2.groups.io; Li, Walon <walon.li@hpe.com>
Cc: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; lersek@redhat.com; ray.ni@intel.com; hao.a.wu@intel.com
Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder

Walon:
  Can you specify the detail reason why BootManagerMenu should be placed at end of BootOrder?

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+71766+4905953+8761045@groups.io
> <bounce+27952+71766+4905953+8761045@groups.io> 代表 Li, Walon
> 发送时间: 2021年2月18日 11:26
> 收件人: devel@edk2.groups.io
> 抄送: walon.li@hpe.com; sunnywang@hpe.com; lersek@redhat.com; 
> ray.ni@intel.com; hao.a.wu@intel.com
> 主题: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Put BootMenu 
> at the end of BootOrder
> 
> REF:INVALID URI REMOVED
> ocore.org_show-5Fbug.cgi-3Fid-3D3135&d=DwIFbw&c=C5b8zRQO1miGmBeVZ2LFWg
> &r=nGx4G_nX3rQG_ai3uSb52w&m=4xka-z98KYmCRK888F4f_O1i7tKha1xqkOolDMIoMN
> w&s=G7KN5FPIan9u09Esxr73N0cT6RHiEP7pdQQqikPiss4&e=
> 
> When Boot Menu does not exist in the BootOrder, 
> BmRegisterBootManagerMenu will create one into list. However, it 
> should be put at the "end" of BootOrder instead of "start" of 
> BootOrder. Replace 0 by -1 to adjust order of load options.
> 
> Signed-off-by: Walon Li <walon.li@hpe.com>
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index aff620ad52..26d1fb0ea0 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -2505,7 +2505,7 @@ BmRegisterBootManagerMenu (
>      EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> 
>      );
> 
> 
> 
> -  return EfiBootManagerAddLoadOptionVariable (BootOption, 0);
> 
> +  return EfiBootManagerAddLoadOptionVariable (BootOption, (UINTN) 
> + -1));
> 
>  }
> 
> 
> 
>  /**
> 
> --
> 2.23.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71766): 
> INVALID URI REMOVED
> devel_message_71766&d=DwIFbw&c=C5b8zRQO1miGmBeVZ2LFWg&r=nGx4G_nX3rQG_a
> i3uSb52w&m=4xka-z98KYmCRK888F4f_O1i7tKha1xqkOolDMIoMNw&s=FNeonYnzA5fhg
> h2S6hfP4kY5-gdgPq0eocZbLoguHso&e= Mute This Topic: 
> INVALID URI REMOVED
> 1971_4905953&d=DwIFbw&c=C5b8zRQO1miGmBeVZ2LFWg&r=nGx4G_nX3rQG_ai3uSb52
> w&m=4xka-z98KYmCRK888F4f_O1i7tKha1xqkOolDMIoMNw&s=PHg6v0w7mvUp-SA38Cx9
> dzS9IaedUWvbERQszTLJ3w0&e= Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: 
> INVALID URI REMOVED
> devel_unsub&d=DwIFbw&c=C5b8zRQO1miGmBeVZ2LFWg&r=nGx4G_nX3rQG_ai3uSb52w
> &m=4xka-z98KYmCRK888F4f_O1i7tKha1xqkOolDMIoMNw&s=wDph98KE_DgEz55q-XSWy
> -RmRDLGolPPOyZFgp01r0Y&e=
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
> 

Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder

[Laszlo Ersek]

On 02/19/21 02:33, Li, Walon wrote:
> Hi Liming,
> 
> As edk2 design, any new boot options should be put at the end of BootOrder because these are NEW . That means system should "append" BootOrder instead of override original order.
> For example, if system has three boot options currently - Boot0001, Boot0002, Boot0003 and then one new option - Boot0000 will be added. The order should become Boot0001,Boot0002,Boot0003,Boot0000. However, in this case, BootmanagerMenu doesn't follow this rule. We set "zero" priority so system would put BootManagerMenu boot option at start.
> This case is a corner case because the symptom only be gotten when user delete BootManagerMenu on OS or EFI shell. But it's a possible case. For keeping behavior consistent, we should keep BootManagerMenu option behavior as same as others boot option.
> 
> Thanks
> Walon
> 
> -----Original Message-----
> From: gaoliming <gaoliming@byosoft.com.cn> 
> Sent: Friday, February 19, 2021 8:59 AM
> To: devel@edk2.groups.io; Li, Walon <walon.li@hpe.com>
> Cc: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; lersek@redhat.com; ray.ni@intel.com; hao.a.wu@intel.com
> Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder
> 
> Walon:
>   Can you specify the detail reason why BootManagerMenu should be placed at end of BootOrder?

In addition to Walon's answer, I'd like to point out that the current
code doesn't seem to explain why BootManagerMenu was ever placed at the
front.

Of course when we change code, the burden of justification is on the
patch that's being proposed; so I agree with the question.

However, answering that question is easier if the pre-patch code has an
explicit explanation, and that explanation can be shown to be wrong (or
at least to have missed a corner case or similar). If the pre-patch code
is undocumented, then arguing for the new code is more difficult.

Laszlo

回复: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Put BootMenu at the end of BootOrder

[gaoliming]

Walon:
  Thanks for your detail explanation. I would like to include more detail so that we can easily know why we make this change.   

Thanks
Liming
> -----邮件原件-----
> 发件人: Laszlo Ersek <lersek@redhat.com>
> 发送时间: 2021年2月19日 23:52
> 收件人: Li, Walon <walon.li@hpe.com>; gaoliming
> <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
> 抄送: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; ray.ni@intel.com;
> hao.a.wu@intel.com
> 主题: Re: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib: Put
> BootMenu at the end of BootOrder
> 
> On 02/19/21 02:33, Li, Walon wrote:
> > Hi Liming,
> >
> > As edk2 design, any new boot options should be put at the end of BootOrder
> because these are NEW . That means system should "append" BootOrder
> instead of override original order.
> > For example, if system has three boot options currently - Boot0001,
> Boot0002, Boot0003 and then one new option - Boot0000 will be added. The
> order should become Boot0001,Boot0002,Boot0003,Boot0000. However, in
> this case, BootmanagerMenu doesn't follow this rule. We set "zero" priority so
> system would put BootManagerMenu boot option at start.
> > This case is a corner case because the symptom only be gotten when user
> delete BootManagerMenu on OS or EFI shell. But it's a possible case. For
> keeping behavior consistent, we should keep BootManagerMenu option
> behavior as same as others boot option.
> >
> > Thanks
> > Walon
> >
> > -----Original Message-----
> > From: gaoliming <gaoliming@byosoft.com.cn>
> > Sent: Friday, February 19, 2021 8:59 AM
> > To: devel@edk2.groups.io; Li, Walon <walon.li@hpe.com>
> > Cc: Wang, Sunny (HPS SW) <sunnywang@hpe.com>; lersek@redhat.com;
> ray.ni@intel.com; hao.a.wu@intel.com
> > Subject: 回复: [edk2-devel] [PATCH] MdeModulePkg/UefiBootManagerLib:
> Put BootMenu at the end of BootOrder
> >
> > Walon:
> >   Can you specify the detail reason why BootManagerMenu should be
> placed at end of BootOrder?
> 
> In addition to Walon's answer, I'd like to point out that the current
> code doesn't seem to explain why BootManagerMenu was ever placed at the
> front.
> 
> Of course when we change code, the burden of justification is on the
> patch that's being proposed; so I agree with the question.
> 
> However, answering that question is easier if the pre-patch code has an
> explicit explanation, and that explanation can be shown to be wrong (or
> at least to have missed a corner case or similar). If the pre-patch code
> is undocumented, then arguing for the new code is more difficult.
> 
> Laszlo