Thanks.https://github.com/tianocore/edk2/pull/5533
> -----Original Message----- > From: Bi, Dandan <dandan...@intel.com> > Sent: Sunday, April 7, 2024 10:07 AM > To: Tan, Ming <ming....@intel.com>; devel@edk2.groups.io > Cc: Xu, Min M <min.m...@intel.com>; Yao, Jiewen <jiewen....@intel.com>; > POLUDOV, FELIX <fel...@ami.com> > Subject: RE: [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according > to UEFI spec > > Reviewed-by: Dandan Bi <dandan...@intel.com> > > -----Original Message----- > From: Tan, Ming <ming....@intel.com> > Sent: Tuesday, April 2, 2024 4:32 PM > To: devel@edk2.groups.io > Cc: Xu, Min M <min.m...@intel.com>; Yao, Jiewen <jiewen....@intel.com>; Bi, > Dandan <dandan...@intel.com>; POLUDOV, FELIX <fel...@ami.com> > Subject: [PATCH v4] 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() and update > IfrNvData->ListCount to save the change to EFI variable, then HII use > IfrNvData->EFI > variable to control the UI. > > Cc: Min Xu <min.m...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Dandan Bi <dandan...@intel.com> > Cc: Felix Polyudov <fel...@ami.com> > Signed-off-by: Ming Tan <ming....@intel.com> > --- > PR: https://github.com/tianocore/edk2/pull/5411 > > V4: Fix a Cc issue of miss a space. > V3: According to Dandan Bi's feedback, does not call > SecureBootExtractConfigFromVariable() at last, but call it as needed. > And add more code for update IfrNvData->ListCount. > V2: Change code style to pass uncrustify check. > > .../SecureBootConfigImpl.c | 42 +++++++++++-------- > 1 file changed, 25 insertions(+), 17 deletions(-) > > diff --git > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm > pl.c > b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm > pl.c > index 2c11129526..6d4560c39b 100644 > --- > a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm > pl.c > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo > +++ nfigImpl.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; }@@ - > 4764,6 +4763,8 @@ SecureBootCallback ( > L"Only Physical Presence User could delete PK in custom > mode!", > NULL );+ } else {+ > SecureBootExtractConfigFromVariable > (Private, IfrNvData); } } }@@ -4827,6 +4828,7 @@ > SecureBootCallback ( > SECUREBOOT_DELETE_SIGNATURE_LIST_FORM, > OPTION_SIGNATURE_LIST_QUESTION_ID );+ IfrNvData->ListCount = > Private->ListCount; break; //@@ -4851,6 +4853,7 @@ > SecureBootCallback ( > SECUREBOOT_DELETE_SIGNATURE_LIST_FORM, > OPTION_SIGNATURE_LIST_QUESTION_ID );+ IfrNvData->ListCount = > Private->ListCount; break; //@@ -4875,6 +4878,7 @@ > SecureBootCallback ( > SECUREBOOT_DELETE_SIGNATURE_LIST_FORM, > OPTION_SIGNATURE_LIST_QUESTION_ID );+ IfrNvData->ListCount = > Private->ListCount; break; case > SECUREBOOT_DELETE_SIGNATURE_FROM_DBT:@@ -4954,6 +4958,8 @@ > SecureBootCallback ( > L"Only supports DER-encoded X509 certificate, AUTH_2 format data > & > executable EFI image", NULL );+ } else {+ > IfrNvData- > >ListCount = Private->ListCount; } break;@@ -5005,6 +5011,8 > >@@ > SecureBootCallback ( > PromptString, NULL );+ } else {+ > SecureBootExtractConfigFromVariable (Private, IfrNvData); } > break;-- > 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117473): https://edk2.groups.io/g/devel/message/117473 Mute This Topic: https://groups.io/mt/105284072/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
