Reviewed-by: Dandan Bi <dandan...@intel.com>
Thanks, Dandan -----Original Message----- From: Kuo, CindyX <cindyx....@intel.com> Sent: Thursday, April 11, 2024 11:11 AM To: devel@edk2.groups.io Cc: Kuo, CindyX <cindyx....@intel.com>; Yao, Jiewen <jiewen....@intel.com>; Kumar, Rahul R <rahul.r.ku...@intel.com>; Bi, Dandan <dandan...@intel.com>; Tan, Ming <ming....@intel.com>; Chen, Arthur G <arthur.g.c...@intel.com>; Chen, Xiao X <xiao.x.c...@intel.com>; Chen, Tina <tina.c...@intel.com> Subject: [PATCH v3] SecurityPkg/OpalPasswordDxe: Update UI according to UEFI spec REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4735 Should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN call back function. Those APIs are called within OpalHiiSetBrowserData/OpalHiiGetBrowserData which have been used by OpalHii.c. 1. Change callback action from FORM_OPEN to RETRIEVE. 2. Create dummy label with suppressif statement in VFR for form update usage. 3. Add HiiUpdateForm() to force reparsing the IFR binary. Cc: Jiewen Yao <jiewen....@intel.com> Cc: Rahul Kumar <rahul1.ku...@intel.com> Cc: Dandan Bi <dandan...@intel.com> Cc: Ming Tan <ming....@intel.com> Cc: Arthur Chen <arthur.g.c...@intel.com> Cc: Xiao X Chen <xiao.x.c...@intel.com> Cc: Tina Chen <tina.c...@intel.com> Signed-off-by: CindyX Kuo <cindyx....@intel.com> --- .../Tcg/Opal/OpalPassword/OpalDriver.h | 1 + SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c | 84 ++++++++++++++++--- .../Tcg/Opal/OpalPassword/OpalHiiFormValues.h | 6 ++ .../Tcg/Opal/OpalPassword/OpalPasswordDxe.inf | 1 + .../Opal/OpalPassword/OpalPasswordForm.vfr | 8 +- 5 files changed, 87 insertions(+), 13 deletions(-) diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h index 2089bd81b6..1a4671c602 100644 --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.h @@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include <Guid/EventGroup.h> #include <Guid/S3StorageDeviceInitList.h> +#include <Guid/MdeModuleHii.h> #include <Library/UefiLib.h> #include <Library/UefiBootServicesTableLib.h> diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c index 8035f44ebe..47af4fee40 100644 --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c @@ -40,6 +40,7 @@ EFI_HII_HANDLE gHiiPackageListHandle = NULL; // const EFI_GUID gHiiPackageListGuid = PACKAGE_LIST_GUID; const EFI_GUID gHiiSetupVariableGuid = SETUP_VARIABLE_GUID; +const EFI_GUID gOpalSetupFormSetGuid = SETUP_FORMSET_GUID; // // Structure that contains state of the HII @@ -611,10 +612,15 @@ DriverCallback ( EFI_BROWSER_ACTION_REQUEST *ActionRequest ) { - HII_KEY HiiKey; - UINT8 HiiKeyId; - UINT32 PpRequest; - OPAL_DISK *OpalDisk; + HII_KEY HiiKey; + UINT8 HiiKeyId; + UINT32 PpRequest; + OPAL_DISK *OpalDisk; + EFI_STATUS Status; + VOID *StartOpCodeHandle; + VOID *EndOpCodeHandle; + EFI_IFR_GUID_LABEL *StartLabel; + EFI_IFR_GUID_LABEL *EndLabel; if (ActionRequest != NULL) { *ActionRequest = EFI_BROWSER_ACTION_REQUEST_NONE; @@ -632,15 +638,69 @@ DriverCallback ( HiiKey.Raw = QuestionId; HiiKeyId = (UINT8)HiiKey.KeyBits.Id; - if (Action == EFI_BROWSER_ACTION_FORM_OPEN) { - switch (HiiKeyId) { - case HII_KEY_ID_VAR_SUPPORTED_DISKS: - DEBUG ((DEBUG_INFO, "HII_KEY_ID_VAR_SUPPORTED_DISKS\n")); - return HiiPopulateMainMenuForm (); + if (Action == EFI_BROWSER_ACTION_RETRIEVE) { + if ((HiiKeyId == HII_KEY_ID_VAR_SUPPORTED_DISKS) || (HiiKeyId == HII_KEY_ID_VAR_SELECTED_DISK_AVAILABLE_ACTIONS)) { + // + // Allocate space for creation of UpdateData Buffer + // + StartOpCodeHandle = HiiAllocateOpCodeHandle (); + if (StartOpCodeHandle == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + EndOpCodeHandle = HiiAllocateOpCodeHandle (); + if (EndOpCodeHandle == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + // + // Create Hii Extend Label OpCode as the start opcode + // + StartLabel = (EFI_IFR_GUID_LABEL *)HiiCreateGuidOpCode (StartOpCodeHandle, &gEfiIfrTianoGuid, NULL, sizeof (EFI_IFR_GUID_LABEL)); + StartLabel->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL; + + // + // Create Hii Extend Label OpCode as the end opcode + // + EndLabel = (EFI_IFR_GUID_LABEL *)HiiCreateGuidOpCode (EndOpCodeHandle, &gEfiIfrTianoGuid, NULL, sizeof (EFI_IFR_GUID_LABEL)); + EndLabel->ExtendOpCode = EFI_IFR_EXTEND_OP_LABEL; + + switch (HiiKeyId) { + case HII_KEY_ID_VAR_SUPPORTED_DISKS: + DEBUG ((DEBUG_INFO, "HII_KEY_ID_VAR_SUPPORTED_DISKS\n")); + Status = HiiPopulateMainMenuForm (); + + StartLabel->Number = OPAL_MAIN_MENU_LABEL_START; + EndLabel->Number = OPAL_MAIN_MENU_LABEL_END; + HiiUpdateForm ( + gHiiPackageListHandle, + (EFI_GUID *)&gOpalSetupFormSetGuid, + FORMID_VALUE_MAIN_MENU, + StartOpCodeHandle, + EndOpCodeHandle + ); + break; + + case HII_KEY_ID_VAR_SELECTED_DISK_AVAILABLE_ACTIONS: + DEBUG ((DEBUG_INFO, "HII_KEY_ID_VAR_SELECTED_DISK_AVAILABLE_ACTIONS\n")); + Status = HiiPopulateDiskInfoForm (); + + StartLabel->Number = OPAL_DISK_INFO_LABEL_START; + EndLabel->Number = OPAL_DISK_INFO_LABEL_END; + HiiUpdateForm ( + gHiiPackageListHandle, + (EFI_GUID *)&gOpalSetupFormSetGuid, + FORMID_VALUE_DISK_INFO_FORM_MAIN, + StartOpCodeHandle, + EndOpCodeHandle + ); + break; + } + + HiiFreeOpCodeHandle (StartOpCodeHandle); + HiiFreeOpCodeHandle (EndOpCodeHandle); - case HII_KEY_ID_VAR_SELECTED_DISK_AVAILABLE_ACTIONS: - DEBUG ((DEBUG_INFO, "HII_KEY_ID_VAR_SELECTED_DISK_AVAILABLE_ACTIONS\n")); - return HiiPopulateDiskInfoForm (); + return Status; } } else if (Action == EFI_BROWSER_ACTION_CHANGING) { switch (HiiKeyId) { diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormValues.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormValues.h index ab6957fc6f..0e098854ba 100644 --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormValues.h +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormValues.h @@ -96,6 +96,12 @@ typedef struct { #define HII_KEY(id) HII_KEY_WITH_INDEX(id, 0) +/* Label */ +#define OPAL_MAIN_MENU_LABEL_START 0x6100 +#define OPAL_MAIN_MENU_LABEL_END 0x6101 +#define OPAL_DISK_INFO_LABEL_START 0x6200 +#define OPAL_DISK_INFO_LABEL_END 0x6201 + #define PACKAGE_LIST_GUID { 0xf0308176, 0x9058, 0x4153, { 0x93, 0x3d, 0xda, 0x2f, 0xdc, 0xc8, 0x3e, 0x44 } } /* {410483CF-F4F9-4ece-848A-1958FD31CEB7} */ diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordDxe.inf b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordDxe.inf index 87519198c0..89e72a74bc 100644 --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordDxe.inf +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordDxe.inf @@ -69,6 +69,7 @@ [Guids] gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event gS3StorageDeviceInitListGuid ## SOMETIMES_PRODUCES ## UNDEFINED + gEfiIfrTianoGuid ## CONSUMES [Pcd] gEfiSecurityPkgTokenSpaceGuid.PcdSkipOpalPasswordPrompt ## CONSUMES diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordForm.vfr b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordForm.vfr index f0d3e220b2..a1049686ff 100644 --- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordForm.vfr +++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordForm.vfr @@ -25,8 +25,11 @@ formset form formid = FORMID_VALUE_MAIN_MENU, title = STRING_TOKEN(STR_OPAL); - //CONFIG_VARIABLE(HII_KEY(HII_KEY_ID_VAR_SUPPORTED_DISKS), SupportedDisks, 0x0, 0xFFFF); suppressif TRUE; + label OPAL_MAIN_MENU_LABEL_START; + label OPAL_MAIN_MENU_LABEL_END; + + //CONFIG_VARIABLE(HII_KEY(HII_KEY_ID_VAR_SUPPORTED_DISKS), + SupportedDisks, 0x0, 0xFFFF); numeric name = SupportedDisks, varid = OpalHiiConfig.SupportedDisks, @@ -149,6 +152,9 @@ form formid = FORMID_VALUE_DISK_INFO_FORM_MAIN, title = STRING_TOKEN(STR_OPAL); suppressif TRUE; + label OPAL_DISK_INFO_LABEL_START; + label OPAL_DISK_INFO_LABEL_END; + numeric name = SelectedDiskAvailableActions, varid = OpalHiiConfig.SelectedDiskAvailableActions, -- 2.44.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117632): https://edk2.groups.io/g/devel/message/117632 Mute This Topic: https://groups.io/mt/105456188/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
