Re: [edk2-devel] [PATCH v2] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec

["Yao, Jiewen" <jiewen.yao@intel.com>]

Thanks for the update.

First, would you please clarify which test you have done for this patch set.
Have you tested all previous function to ensure it still works?

Second, would you please clarify if there is any compatibility issue to follow the new UEFI 2.10?
For example, what if the core HII is still UEFI 2.9? would that still work?

Third, because I am not HII expert, I would like to have HII expert to comment the HII/Browser related change.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: Tan, Ming <ming.tan@intel.com>
> Sent: Tuesday, February 27, 2024 10:59 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH v2] SecurityPkg/SecureBootConfigDxe: Update UI according to
> UEFI spec
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4713
> 
> In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
> EFI_BROWSER_ACTION_FORM_OPEN:
> NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used
> with
> this browser action because question values have not been retrieved yet.
> 
> So should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN
> call back function.
> 
> Now call SecureBootExtractConfigFromVariable() to save the change to EFI
> variable, then HII use EFI variable to control the UI.
> 
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Ming Tan <ming.tan@intel.com>
> ---
>   V2: Change code style to pass uncrustify check.
> 
>  .../SecureBootConfigImpl.c                    | 37 ++++++++++---------
>  1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> index 2c11129526..e2e61d1e07 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> @@ -3366,6 +3366,8 @@ SecureBootExtractConfigFromVariable (
>      ConfigData->FileEnrollType = UNKNOWN_FILE_TYPE;
> 
>    }
> 
> 
> 
> +  ConfigData->ListCount = Private->ListCount;
> 
> +
> 
>    //
> 
>    // If it is Physical Presence User, set the PhysicalPresent to true.
> 
>    //
> 
> @@ -4541,12 +4543,13 @@ SecureBootCallback (
>    EFI_HII_POPUP_PROTOCOL          *HiiPopup;
> 
>    EFI_HII_POPUP_SELECTION         UserSelection;
> 
> 
> 
> -  Status             = EFI_SUCCESS;
> 
> -  SecureBootEnable   = NULL;
> 
> -  SecureBootMode     = NULL;
> 
> -  SetupMode          = NULL;
> 
> -  File               = NULL;
> 
> -  EnrollKeyErrorCode = None_Error;
> 
> +  Status               = EFI_SUCCESS;
> 
> +  SecureBootEnable     = NULL;
> 
> +  SecureBootMode       = NULL;
> 
> +  SetupMode            = NULL;
> 
> +  File                 = NULL;
> 
> +  EnrollKeyErrorCode   = None_Error;
> 
> +  GetBrowserDataResult = FALSE;
> 
> 
> 
>    if ((This == NULL) || (Value == NULL) || (ActionRequest == NULL)) {
> 
>      return EFI_INVALID_PARAMETER;
> 
> @@ -4565,15 +4568,12 @@ SecureBootCallback (
>      return EFI_OUT_OF_RESOURCES;
> 
>    }
> 
> 
> 
> -  GetBrowserDataResult = HiiGetBrowserData
> (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize,
> (UINT8 *)IfrNvData);
> 
> -
> 
>    if (Action == EFI_BROWSER_ACTION_FORM_OPEN) {
> 
>      if (QuestionId == KEY_SECURE_BOOT_MODE) {
> 
>        //
> 
>        // Update secure boot strings when opening this form
> 
>        //
> 
> -      Status = UpdateSecureBootString (Private);
> 
> -      SecureBootExtractConfigFromVariable (Private, IfrNvData);
> 
> +      Status                 = UpdateSecureBootString (Private);
> 
>        mIsEnterSecureBootForm = TRUE;
> 
>      } else {
> 
>        //
> 
> @@ -4587,23 +4587,22 @@ SecureBootCallback (
>            (QuestionId == KEY_SECURE_BOOT_DBT_OPTION))
> 
>        {
> 
>          CloseEnrolledFile (Private->FileContext);
> 
> -      } else if (QuestionId == KEY_SECURE_BOOT_DELETE_ALL_LIST) {
> 
> -        //
> 
> -        // Update ListCount field in varstore
> 
> -        // Button "Delete All Signature List" is
> 
> -        // enable when ListCount is greater than 0.
> 
> -        //
> 
> -        IfrNvData->ListCount = Private->ListCount;
> 
>        }
> 
>      }
> 
> 
> 
>      goto EXIT;
> 
>    }
> 
> 
> 
> +  GetBrowserDataResult = HiiGetBrowserData
> (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize,
> (UINT8 *)IfrNvData);
> 
> +
> 
>    if (Action == EFI_BROWSER_ACTION_RETRIEVE) {
> 
>      Status = EFI_UNSUPPORTED;
> 
>      if (QuestionId == KEY_SECURE_BOOT_MODE) {
> 
>        if (mIsEnterSecureBootForm) {
> 
> +        if (GetBrowserDataResult) {
> 
> +          SecureBootExtractConfigFromVariable (Private, IfrNvData);
> 
> +        }
> 
> +
> 
>          Value->u8 = SECURE_BOOT_MODE_STANDARD;
> 
>          Status    = EFI_SUCCESS;
> 
>        }
> 
> @@ -5179,6 +5178,10 @@ SecureBootCallback (
>      }
> 
>    }
> 
> 
> 
> +  if (GetBrowserDataResult) {
> 
> +    SecureBootExtractConfigFromVariable (Private, IfrNvData);
> 
> +  }
> 
> +
> 
>  EXIT:
> 
> 
> 
>    if (!EFI_ERROR (Status) && GetBrowserDataResult) {
> 
> --
> 2.31.1.windows.1



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