[edk2-devel] [PATCH] SecurityPkg/OpalPasswordDxe: Update UI according to UEFI spec

["Cindy Kuo" <cindyx.kuo@intel.com>]

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.yao@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Ming Tan <ming.tan@intel.com>
Cc: Arthur Chen <arthur.g.chen@intel.com>
Cc: Xiao X Chen <xiao.x.chen@intel.com>
Cc: Tina Chen <tina.chen@intel.com>
Signed-off-by: CindyX Kuo <cindyx.kuo@intel.com>
---
 .../Tcg/Opal/OpalPassword/OpalDriver.h        |  1 +
 SecurityPkg/Tcg/Opal/OpalPassword/OpalHii.c   | 76 +++++++++++++++++--
 .../Tcg/Opal/OpalPassword/OpalHiiFormValues.h |  5 ++
 .../Tcg/Opal/OpalPassword/OpalPasswordDxe.inf |  1 +
 .../Opal/OpalPassword/OpalPasswordForm.vfr    | 10 +++
 5 files changed, 86 insertions(+), 7 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..b513c053b9 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,16 +638,72 @@ DriverCallback (
   HiiKey.Raw = QuestionId;
   HiiKeyId   = (UINT8)HiiKey.KeyBits.Id;
 
-  if (Action == EFI_BROWSER_ACTION_FORM_OPEN) {
+  if (Action == EFI_BROWSER_ACTION_RETRIEVE) {
+    //
+    // 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"));
-        return HiiPopulateMainMenuForm ();
+        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"));
-        return HiiPopulateDiskInfoForm ();
+        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;
+
+      default:
+        Status = EFI_UNSUPPORTED;
+        break;
     }
+
+    HiiFreeOpCodeHandle (StartOpCodeHandle);
+    HiiFreeOpCodeHandle (EndOpCodeHandle);
+
+    return Status;
+
   } else if (Action == EFI_BROWSER_ACTION_CHANGING) {
     switch (HiiKeyId) {
       case HII_KEY_ID_GOTO_DISK_INFO:
diff --git a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormValues.h b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormValues.h
index ab6957fc6f..eec41caf01 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormValues.h
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalHiiFormValues.h
@@ -96,6 +96,11 @@ typedef struct {
 
 #define HII_KEY(id)  HII_KEY_WITH_INDEX(id, 0)
 
+#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..a03c19af11 100644
--- a/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordForm.vfr
+++ b/SecurityPkg/Tcg/Opal/OpalPassword/OpalPasswordForm.vfr
@@ -25,6 +25,11 @@ formset
 form formid = FORMID_VALUE_MAIN_MENU,
     title  = STRING_TOKEN(STR_OPAL);
 
+    suppressif TRUE;
+        label OPAL_MAIN_MENU_LABEL_START;
+        label OPAL_MAIN_MENU_LABEL_END;
+    endif;
+
     //CONFIG_VARIABLE(HII_KEY(HII_KEY_ID_VAR_SUPPORTED_DISKS), SupportedDisks, 0x0, 0xFFFF);
     suppressif TRUE;
         numeric
@@ -148,6 +153,11 @@ endform;  // MAIN MENU FORM
 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;
+    endif;
+
     suppressif TRUE;
         numeric
             name    = SelectedDiskAvailableActions,
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117562): https://edk2.groups.io/g/devel/message/117562
Mute This Topic: https://groups.io/mt/105434774/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-