public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Cindy Kuo" <cindyx.kuo@intel.com>
To: devel@edk2.groups.io
Cc: Cindy Kuo <cindyx.kuo@intel.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Rahul Kumar <rahul1.kumar@intel.com>,
	Dandan Bi <dandan.bi@intel.com>, Ming Tan <ming.tan@intel.com>,
	Arthur Chen <arthur.g.chen@intel.com>,
	Xiao X Chen <xiao.x.chen@intel.com>,
	Tina Chen <tina.chen@intel.com>
Subject: [edk2-devel] [PATCH v3] SecurityPkg/OpalPasswordDxe: Update UI according to UEFI spec
Date: Thu, 11 Apr 2024 11:10:49 +0800	[thread overview]
Message-ID: <46e2b8943750c127746125f0265b91ab91a0a859.1712804652.git.cindyx.kuo@intel.com> (raw)

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   | 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 (#117608): https://edk2.groups.io/g/devel/message/117608
Mute This Topic: https://groups.io/mt/105456188/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



             reply	other threads:[~2024-04-11  3:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-11  3:10 Cindy Kuo [this message]
2024-04-11 11:15 ` [edk2-devel] [PATCH v3] SecurityPkg/OpalPasswordDxe: Update UI according to UEFI spec Dandan Bi
2024-04-11 15:45   ` Yao, Jiewen
2024-04-12  3:24     ` Tina Chen
2024-04-12  4:05       ` Yao, Jiewen
2024-04-12  7:05         ` Cindy Kuo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46e2b8943750c127746125f0265b91ab91a0a859.1712804652.git.cindyx.kuo@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox