From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 99EF381F5C for ; Sun, 26 Feb 2017 23:19:08 -0800 (PST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP; 26 Feb 2017 23:19:08 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,213,1484035200"; d="scan'208";a="1102660813" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga001.jf.intel.com with ESMTP; 26 Feb 2017 23:19:05 -0800 Received: from fmsmsx151.amr.corp.intel.com (10.18.125.4) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 26 Feb 2017 23:18:53 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX151.amr.corp.intel.com (10.18.125.4) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 26 Feb 2017 23:18:53 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.59]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.204]) with mapi id 14.03.0248.002; Mon, 27 Feb 2017 15:18:43 +0800 From: "Ni, Ruiyu" To: "Bi, Dandan" , "edk2-devel@lists.01.org" CC: "Dong, Eric" , "Zeng, Star" Thread-Topic: [patch] MdeModulePkg/BMMUiLib: Replace same logic with API in UefiBootManagerLib Thread-Index: AQHSkMlNk+q6SWjxzk2zY/ujMNnYvqF8cXuw Date: Mon, 27 Feb 2017 07:18:42 +0000 Deferred-Delivery: Mon, 27 Feb 2017 07:18:00 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5B8B4E46@SHSMSX104.ccr.corp.intel.com> References: <1488179688-22428-1-git-send-email-dandan.bi@intel.com> In-Reply-To: <1488179688-22428-1-git-send-email-dandan.bi@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [patch] MdeModulePkg/BMMUiLib: Replace same logic with API in UefiBootManagerLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Feb 2017 07:19:08 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-2022-jp" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ruiyu Ni Thanks/Ray > -----Original Message----- > From: Bi, Dandan > Sent: Monday, February 27, 2017 3:15 PM > To: edk2-devel@lists.01.org > Cc: Dong, Eric ; Ni, Ruiyu ; Zen= g, > Star > Subject: [patch] MdeModulePkg/BMMUiLib: Replace same logic with API in > UefiBootManagerLib >=20 > Use the API=1B$B!!=1B(BEfiBootManagerDeleteLoadOptionVariable in > UefiBootManagerLib=1B$B!!=1B(Bto replace the same logic in function > Var_DelBootOption/Var_DelDriverOption. > This can make code clean and prevent potential bugs. >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D391 >=20 > Cc: Eric Dong > Cc: Ruiyu Ni > Cc: Star Zeng > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Dandan Bi > --- > .../BootMaintenanceManager.h | 37 +--- > .../Library/BootMaintenanceManagerUiLib/Variable.c | 202 +--------------= ----- > - > 2 files changed, 11 insertions(+), 228 deletions(-) >=20 > diff --git > a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanc > eManager.h > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanc > eManager.h > index 532b75b..a8d7a0f 100644 > --- > a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanc > eManager.h > +++ > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanc > eMa > +++ nager.h > @@ -631,12 +631,10 @@ Var_UpdateBootOption ( > IN BMM_CALLBACK_DATA *CallbackData > ); >=20 > /** > Delete Boot Option that represent a Deleted state in BootOptionMenu. > - After deleting this boot option, call Var_ChangeBootOrder to > - make sure BootOrder is in valid state. >=20 > @retval EFI_SUCCESS If all boot load option EFI Variables correspond= ing to > BM_LOAD_CONTEXT marked for deletion is deleted > @return Others If failed to update the "BootOrder" variable aft= er > deletion. >=20 > @@ -645,25 +643,10 @@ EFI_STATUS > Var_DelBootOption ( > VOID > ); >=20 > /** > - After any operation on Boot####, there will be a discrepancy in BootOr= der. > - Since some are missing but in BootOrder, while some are present but ar= e > - not reflected by BootOrder. Then a function rebuild BootOrder from > - scratch by content from BootOptionMenu is needed. > - > - @retval EFI_SUCCESS The boot order is updated successfully. > - @return other than EFI_SUCCESS if failed to change the "BootOrder" EF= I > Variable. > - > -**/ > -EFI_STATUS > -Var_ChangeBootOrder ( > - VOID > - ); > - > -/** > This function create a currently loaded Drive Option from > the BMM. It then appends this Driver Option to the end of > the "DriverOrder" list. It append this Driver Opotion to the end > of DriverOptionMenu. >=20 > @@ -685,13 +668,11 @@ Var_UpdateDriverOption ( > IN UINT16 *OptionalData, > IN UINT8 ForceReconnect > ); >=20 > /** > - Delete Load Option that represent a Deleted state in BootOptionMenu. > - After deleting this Driver option, call Var_ChangeDriverOrder to > - make sure DriverOrder is in valid state. > + Delete Load Option that represent a Deleted state in DriverOptionMenu. >=20 > @retval EFI_SUCCESS Load Option is successfully updated. > @return Other value than EFI_SUCCESS if failed to update "Driver Order= " > EFI > Variable. >=20 > @@ -700,26 +681,10 @@ EFI_STATUS > Var_DelDriverOption ( > VOID > ); >=20 > /** > - After any operation on Driver####, there will be a discrepancy in > - DriverOrder. Since some are missing but in DriverOrder, while some > - are present but are not reflected by DriverOrder. Then a function > - rebuild DriverOrder from scratch by content from DriverOptionMenu is > - needed. > - > - @retval EFI_SUCCESS The driver order is updated successfully. > - @return other than EFI_SUCCESS if failed to set the "DriverOrder" EFI > Variable. > - > -**/ > -EFI_STATUS > -Var_ChangeDriverOrder ( > - VOID > - ); > - > -/** > This function delete and build multi-instance device path ConIn > console device. >=20 > @retval EFI_SUCCESS The function complete successfully. > @return The EFI variable can not be saved. See gRT->SetVariable for de= tail > return information. > diff --git > a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > index 746b233..c1c55b0 100644 > --- a/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > +++ b/MdeModulePkg/Library/BootMaintenanceManagerUiLib/Variable.c > @@ -1,9 +1,9 @@ > /** @file > Variable operation that will be used by bootmaint >=20 > -Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.
> This program and the accompanying materials are licensed and made > available under the terms and conditions of the BSD License which > accompanies this distribution. The full text of the license may be found= at > http://opensource.org/licenses/bsd-license.php >=20 > @@ -14,12 +14,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY KIND, EITHER EXPRESS OR IMPLIED. >=20 > #include "BootMaintenanceManager.h" >=20 > /** > Delete Boot Option that represent a Deleted state in BootOptionMenu. > - After deleting this boot option, call Var_ChangeBootOrder to > - make sure BootOrder is in valid state. >=20 > @retval EFI_SUCCESS If all boot load option EFI Variables correspond= ing to > BM_LOAD_CONTEXT marked for deletion is deleted. > @retval EFI_NOT_FOUND If can not find the boot option want to be > deleted. > @return Others If failed to update the "BootOrder" variable aft= er > deletion. > @@ -30,16 +28,14 @@ Var_DelBootOption ( > VOID > ) > { > BM_MENU_ENTRY *NewMenuEntry; > BM_LOAD_CONTEXT *NewLoadContext; > - UINT16 BootString[10]; > EFI_STATUS Status; > UINTN Index; > UINTN Index2; >=20 > - Status =3D EFI_SUCCESS; > Index2 =3D 0; > for (Index =3D 0; Index < BootOptionMenu.MenuNumber; Index++) { > NewMenuEntry =3D BOpt_GetMenuEntry (&BootOptionMenu, (Index - > Index2)); > if (NULL =3D=3D NewMenuEntry) { > return EFI_NOT_FOUND; > @@ -48,18 +44,14 @@ Var_DelBootOption ( > NewLoadContext =3D (BM_LOAD_CONTEXT *) NewMenuEntry- > >VariableContext; > if (!NewLoadContext->Deleted) { > continue; > } >=20 > - UnicodeSPrint ( > - BootString, > - sizeof (BootString), > - L"Boot%04x", > - NewMenuEntry->OptionNumber > - ); > - > - EfiLibDeleteVariable (BootString, &gEfiGlobalVariableGuid); > + Status =3D EfiBootManagerDeleteLoadOptionVariable (NewMenuEntry- > >OptionNumber,LoadOptionTypeBoot); > + if (EFI_ERROR (Status)) { > + return Status; > + } > Index2++; > // > // If current Load Option is the same as BootNext, > // must delete BootNext in order to make sure > // there will be no panic on next boot @@ -73,106 +65,15 @@ > Var_DelBootOption ( > NewMenuEntry =3D NULL; > } >=20 > BootOptionMenu.MenuNumber -=3D Index2; >=20 > - Status =3D Var_ChangeBootOrder (); > - return Status; > -} > - > -/** > - After any operation on Boot####, there will be a discrepancy in BootOr= der. > - Since some are missing but in BootOrder, while some are present but ar= e > - not reflected by BootOrder. Then a function rebuild BootOrder from > - scratch by content from BootOptionMenu is needed. > - > - > - > - > - @retval EFI_SUCCESS The boot order is updated successfully. > - @return EFI_STATUS other than EFI_SUCCESS if failed to > - Set the "BootOrder" EFI Variable. > - > -**/ > -EFI_STATUS > -Var_ChangeBootOrder ( > - VOID > - ) > -{ > - > - EFI_STATUS Status; > - BM_MENU_ENTRY *NewMenuEntry; > - UINT16 *BootOrderList; > - UINT16 *BootOrderListPtr; > - UINTN BootOrderListSize; > - UINTN Index; > - > - BootOrderList =3D NULL; > - BootOrderListSize =3D 0; > - // > - // First check whether BootOrder is present in current configuration > - // > - GetEfiGlobalVariable2 (L"BootOrder", (VOID **) &BootOrderList, > &BootOrderListSize); > - > - // > - // If exists, delete it to hold new BootOrder > - // > - if (BootOrderList !=3D NULL) { > - EfiLibDeleteVariable (L"BootOrder", &gEfiGlobalVariableGuid); > - FreePool (BootOrderList); > - BootOrderList =3D NULL; > - } > - // > - // Maybe here should be some check method to ensure that > - // no new added boot options will be added > - // but the setup engine now will give only one callback > - // that is to say, user are granted only one chance to > - // decide whether the boot option will be added or not > - // there should be no indictor to show whether this > - // is a "new" boot option > - // > - BootOrderListSize =3D BootOptionMenu.MenuNumber; > - > - if (BootOrderListSize > 0) { > - BootOrderList =3D AllocateZeroPool (BootOrderListSize * sizeof (UINT= 16)); > - ASSERT (BootOrderList !=3D NULL); > - BootOrderListPtr =3D BootOrderList; > - > - // > - // Get all current used Boot#### from BootOptionMenu. > - // OptionNumber in each BM_LOAD_OPTION is really its > - // #### value. > - // > - for (Index =3D 0; Index < BootOrderListSize; Index++) { > - NewMenuEntry =3D BOpt_GetMenuEntry (&BootOptionMenu, Index); > - *BootOrderList =3D (UINT16) NewMenuEntry->OptionNumber; > - BootOrderList++; > - } > - > - BootOrderList =3D BootOrderListPtr; > - > - // > - // After building the BootOrderList, write it back > - // > - Status =3D gRT->SetVariable ( > - L"BootOrder", > - &gEfiGlobalVariableGuid, > - VAR_FLAG, > - BootOrderListSize * sizeof (UINT16), > - BootOrderList > - ); > - if (EFI_ERROR (Status)) { > - return Status; > - } > - } > return EFI_SUCCESS; > } >=20 > /** > - Delete Load Option that represent a Deleted state in BootOptionMenu. > - After deleting this Driver option, call Var_ChangeDriverOrder to > - make sure DriverOrder is in valid state. > + Delete Load Option that represent a Deleted state in DriverOptionMenu. >=20 > @retval EFI_SUCCESS Load Option is successfully updated. > @retval EFI_NOT_FOUND Fail to find the driver option want to be > deleted. > @return Other value than EFI_SUCCESS if failed to update "Driver Order= " > EFI > Variable. > @@ -183,16 +84,14 @@ Var_DelDriverOption ( > VOID > ) > { > BM_MENU_ENTRY *NewMenuEntry; > BM_LOAD_CONTEXT *NewLoadContext; > - UINT16 DriverString[12]; > EFI_STATUS Status; > UINTN Index; > UINTN Index2; >=20 > - Status =3D EFI_SUCCESS; > Index2 =3D 0; > for (Index =3D 0; Index < DriverOptionMenu.MenuNumber; Index++) { > NewMenuEntry =3D BOpt_GetMenuEntry (&DriverOptionMenu, (Index - > Index2)); > if (NULL =3D=3D NewMenuEntry) { > return EFI_NOT_FOUND; > @@ -200,105 +99,24 @@ Var_DelDriverOption ( >=20 > NewLoadContext =3D (BM_LOAD_CONTEXT *) NewMenuEntry- > >VariableContext; > if (!NewLoadContext->Deleted) { > continue; > } > + Status =3D EfiBootManagerDeleteLoadOptionVariable (NewMenuEntry- > >OptionNumber,LoadOptionTypeDriver); > + if (EFI_ERROR (Status)) { > + return Status; > + } >=20 > - UnicodeSPrint ( > - DriverString, > - sizeof (DriverString), > - L"Driver%04x", > - NewMenuEntry->OptionNumber > - ); > - > - EfiLibDeleteVariable (DriverString, &gEfiGlobalVariableGuid); > Index2++; >=20 > RemoveEntryList (&NewMenuEntry->Link); > BOpt_DestroyMenuEntry (NewMenuEntry); > NewMenuEntry =3D NULL; > } >=20 > DriverOptionMenu.MenuNumber -=3D Index2; >=20 > - Status =3D Var_ChangeDriverOrder (); > - return Status; > -} > - > -/** > - After any operation on Driver####, there will be a discrepancy in > - DriverOrder. Since some are missing but in DriverOrder, while some > - are present but are not reflected by DriverOrder. Then a function > - rebuild DriverOrder from scratch by content from DriverOptionMenu is > - needed. > - > - @retval EFI_SUCCESS The driver order is updated successfully. > - @return Other status than EFI_SUCCESS if failed to set the "DriverOrd= er" > EFI Variable. > - > -**/ > -EFI_STATUS > -Var_ChangeDriverOrder ( > - VOID > - ) > -{ > - EFI_STATUS Status; > - BM_MENU_ENTRY *NewMenuEntry; > - UINT16 *DriverOrderList; > - UINT16 *DriverOrderListPtr; > - UINTN DriverOrderListSize; > - UINTN Index; > - > - DriverOrderList =3D NULL; > - DriverOrderListSize =3D 0; > - > - // > - // First check whether DriverOrder is present in current configuration > - // > - GetEfiGlobalVariable2 (L"DriverOrder", (VOID **) &DriverOrderList, > &DriverOrderListSize); > - // > - // If exists, delete it to hold new DriverOrder > - // > - if (DriverOrderList !=3D NULL) { > - EfiLibDeleteVariable (L"DriverOrder", &gEfiGlobalVariableGuid); > - FreePool (DriverOrderList); > - DriverOrderList =3D NULL; > - } > - > - DriverOrderListSize =3D DriverOptionMenu.MenuNumber; > - > - if (DriverOrderListSize > 0) { > - DriverOrderList =3D AllocateZeroPool (DriverOrderListSize * sizeof (= UINT16)); > - ASSERT (DriverOrderList !=3D NULL); > - DriverOrderListPtr =3D DriverOrderList; > - > - // > - // Get all current used Driver#### from DriverOptionMenu. > - // OptionNumber in each BM_LOAD_OPTION is really its > - // #### value. > - // > - for (Index =3D 0; Index < DriverOrderListSize; Index++) { > - NewMenuEntry =3D BOpt_GetMenuEntry (&DriverOptionMenu, Index)= ; > - *DriverOrderList =3D (UINT16) NewMenuEntry->OptionNumber; > - DriverOrderList++; > - } > - > - DriverOrderList =3D DriverOrderListPtr; > - > - // > - // After building the DriverOrderList, write it back > - // > - Status =3D gRT->SetVariable ( > - L"DriverOrder", > - &gEfiGlobalVariableGuid, > - VAR_FLAG, > - DriverOrderListSize * sizeof (UINT16), > - DriverOrderList > - ); > - if (EFI_ERROR (Status)) { > - return Status; > - } > - } > return EFI_SUCCESS; > } >=20 > /** > This function delete and build multi-instance device path for > -- > 1.9.5.msysgit.1