The specific ask is: does the change cause any flexibility lost in HiiConfigAccess implementation? E.g.: something was possible to be done in HiiConfigAccess implementation even for efivarstore but cannot be done with this change.
> -----Original Message----- > From: Ni, Ray > Sent: Friday, August 4, 2023 2:01 PM > To: devel@edk2.groups.io; Bi, Dandan <dandan...@intel.com>; Gao, Liming > <gaolim...@byosoft.com.cn>; Dong, Eric <eric.d...@intel.com>; Rothman, > Michael A <michael.a.roth...@intel.com> > Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load > storage via GetVariable for EfiVarStore > > + @Rothman, Michael A who designed UEFI HII. > > The patch moves the variable access from implementation of ConfigAccess > protocol to Setup driver for efivarstore. > Is it a valid assumption? > > Thanks, > Ray > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dandan > > Bi > > Sent: Thursday, August 3, 2023 12:53 PM > > To: Gao, Liming <gaolim...@byosoft.com.cn>; Dong, Eric > > <eric.d...@intel.com>; devel@edk2.groups.io > > Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load > > storage via GetVariable for EfiVarStore > > > > Hi Liming, > > > > Yes, with this change, the performance is better than before. > > Especially for the big formset with lots of configuration, it has better use > > experience when loading the formset to display. > > > > Could you help review this patch? Thanks. > > > > > > Thanks, > > Dandan > > -----Original Message----- > > From: gaoliming <gaolim...@byosoft.com.cn> > > Sent: Wednesday, August 2, 2023 1:08 PM > > To: Dong, Eric <eric.d...@intel.com>; Bi, Dandan <dandan...@intel.com>; > > devel@edk2.groups.io > > Subject: 回复: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via > > GetVariable for EfiVarStore > > > > Dandan: > > Have you collected the performance data for this enhancement? Is the > > updated one better than before? > > > > Thanks > > Liming > > > -----邮件原件----- > > > 发件人: Dong, Eric <eric.d...@intel.com> > > > 发送时间: 2023年7月31日 13:04 > > > 收件人: Bi, Dandan <dandan...@intel.com>; devel@edk2.groups.io > > > 抄送: Gao, Liming <gaolim...@byosoft.com.cn> > > > 主题: RE: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via > > > GetVariable for EfiVarStore > > > > > > Reviewed-by: Eric Dong <eric.d...@intel.com> > > > > > > -----Original Message----- > > > From: Bi, Dandan <dandan...@intel.com> > > > Sent: Monday, July 31, 2023 8:46 AM > > > To: devel@edk2.groups.io > > > Cc: Gao, Liming <gaolim...@byosoft.com.cn>; Dong, Eric > > > <eric.d...@intel.com> > > > Subject: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via > > > GetVariable for EfiVarStore > > > > > > For EfiVarStore (EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER), it will call > > > ExtractConfig-GetVariable-HiiBlockToConfig-ConfigToBlock when load > > > storage value in LoadStorage function. It's not necessary and costs > > > lots of time > > to do > > > the conversion between config and block. > > > So now enhance it to call GetVariable directly. > > > > > > Cc: Liming Gao <gaolim...@byosoft.com.cn> > > > Cc: Eric Dong <eric.d...@intel.com> > > > Signed-off-by: Dandan Bi <dandan...@intel.com> > > > --- > > > v2: Fix coding style issue. > > > > > > .../Universal/SetupBrowserDxe/Setup.c | 54 +++++++++++-------- > > > 1 file changed, 32 insertions(+), 22 deletions(-) > > > > > > diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c > > > b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c > > > index 5158baf5bd..2f7b11b1aa 100644 > > > --- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c > > > +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c > > > @@ -5634,32 +5634,42 @@ LoadStorage ( > > > ConfigRequest = Storage->ConfigRequest; > > > } > > > > > > - // > > > - // Request current settings from Configuration Driver > > > - // > > > - Status = mHiiConfigRouting->ExtractConfig ( > > > - mHiiConfigRouting, > > > - ConfigRequest, > > > - &Progress, > > > - &Result > > > - ); > > > - > > > - // > > > - // If get value fail, extract default from IFR binary > > > - // > > > - if (EFI_ERROR (Status)) { > > > - ExtractDefault (FormSet, NULL, EFI_HII_DEFAULT_CLASS_STANDARD, > > > FormSetLevel, GetDefaultForStorage, Storage->BrowserStorage, TRUE, > > > TRUE); > > > - } else { > > > + if (Storage->BrowserStorage->Type == > > > + EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER) { > > > // > > > - // Convert Result from <ConfigAltResp> to <ConfigResp> > > > + // Call GetVariable directly for EfiVarStore > > > // > > > - StrPtr = StrStr (Result, L"&GUID="); > > > - if (StrPtr != NULL) { > > > - *StrPtr = L'\0'; > > > + Status = gRT->GetVariable (Storage->BrowserStorage->Name, > > > &(Storage->BrowserStorage->Guid), NULL, (UINTN > > > *)(&(Storage->BrowserStorage->Size)), > > > Storage->BrowserStorage->EditBuffer); > > > + if (EFI_ERROR (Status)) { > > > + ExtractDefault (FormSet, NULL, > > > EFI_HII_DEFAULT_CLASS_STANDARD, > > > + FormSetLevel, GetDefaultForStorage, Storage->BrowserStorage, TRUE, > > > + TRUE); > > > } > > > + } else { > > > + // > > > + // Request current settings from Configuration Driver > > > + // > > > + Status = mHiiConfigRouting->ExtractConfig ( > > > + mHiiConfigRouting, > > > + ConfigRequest, > > > + &Progress, > > > + &Result > > > + ); > > > > > > - Status = ConfigRespToStorage (Storage->BrowserStorage, Result); > > > - FreePool (Result); > > > + // > > > + // If get value fail, extract default from IFR binary > > > + // > > > + if (EFI_ERROR (Status)) { > > > + ExtractDefault (FormSet, NULL, EFI_HII_DEFAULT_CLASS_STANDARD, > > > FormSetLevel, GetDefaultForStorage, Storage->BrowserStorage, TRUE, > > > TRUE); > > > + } else { > > > + // > > > + // Convert Result from <ConfigAltResp> to <ConfigResp> > > > + // > > > + StrPtr = StrStr (Result, L"&GUID="); > > > + if (StrPtr != NULL) { > > > + *StrPtr = L'\0'; > > > + } > > > + > > > + Status = ConfigRespToStorage (Storage->BrowserStorage, Result); > > > + FreePool (Result); > > > + } > > > } > > > > > > Storage->BrowserStorage->ConfigRequest = AllocateCopyPool (StrSize > > > (Storage->ConfigRequest), Storage->ConfigRequest); > > > -- > > > 2.39.1.windows.1 > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107561): https://edk2.groups.io/g/devel/message/107561 Mute This Topic: https://groups.io/mt/100520724/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-