* [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore
@ 2023-07-31 0:46 Dandan Bi
2023-07-31 5:03 ` Dong, Eric
0 siblings, 1 reply; 8+ messages in thread
From: Dandan Bi @ 2023-07-31 0:46 UTC (permalink / raw)
To: devel; +Cc: Liming Gao, Eric Dong
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 <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Dandan Bi <dandan.bi@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 (#107385): https://edk2.groups.io/g/devel/message/107385
Mute This Topic: https://groups.io/mt/100453623/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore
2023-07-31 0:46 [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore Dandan Bi
@ 2023-07-31 5:03 ` Dong, Eric
2023-08-02 5:07 ` [edk2-devel] 回复: " gaoliming via groups.io
0 siblings, 1 reply; 8+ messages in thread
From: Dong, Eric @ 2023-07-31 5:03 UTC (permalink / raw)
To: Bi, Dandan, devel@edk2.groups.io; +Cc: Gao, Liming
Reviewed-by: Eric Dong <eric.dong@intel.com>
-----Original Message-----
From: Bi, Dandan <dandan.bi@intel.com>
Sent: Monday, July 31, 2023 8:46 AM
To: devel@edk2.groups.io
Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@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 <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Dandan Bi <dandan.bi@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 (#107393): https://edk2.groups.io/g/devel/message/107393
Mute This Topic: https://groups.io/mt/100453623/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [edk2-devel] 回复: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore
2023-07-31 5:03 ` Dong, Eric
@ 2023-08-02 5:07 ` gaoliming via groups.io
2023-08-03 4:52 ` [edk2-devel] " Dandan Bi
0 siblings, 1 reply; 8+ messages in thread
From: gaoliming via groups.io @ 2023-08-02 5:07 UTC (permalink / raw)
To: 'Dong, Eric', 'Bi, Dandan', devel
Dandan:
Have you collected the performance data for this enhancement? Is the
updated one better than before?
Thanks
Liming
> -----邮件原件-----
> 发件人: Dong, Eric <eric.dong@intel.com>
> 发送时间: 2023年7月31日 13:04
> 收件人: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
> 抄送: Gao, Liming <gaoliming@byosoft.com.cn>
> 主题: RE: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via
> GetVariable for EfiVarStore
>
> Reviewed-by: Eric Dong <eric.dong@intel.com>
>
> -----Original Message-----
> From: Bi, Dandan <dandan.bi@intel.com>
> Sent: Monday, July 31, 2023 8:46 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric
> <eric.dong@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 <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@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 (#107446): https://edk2.groups.io/g/devel/message/107446
Mute This Topic: https://groups.io/mt/100499712/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore
2023-08-02 5:07 ` [edk2-devel] 回复: " gaoliming via groups.io
@ 2023-08-03 4:52 ` Dandan Bi
2023-08-03 11:05 ` [edk2-devel] 回复: " gaoliming via groups.io
2023-08-04 6:01 ` [edk2-devel] " Ni, Ray
0 siblings, 2 replies; 8+ messages in thread
From: Dandan Bi @ 2023-08-03 4:52 UTC (permalink / raw)
To: Gao, Liming, Dong, Eric, devel@edk2.groups.io
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 <gaoliming@byosoft.com.cn>
Sent: Wednesday, August 2, 2023 1:08 PM
To: Dong, Eric <eric.dong@intel.com>; Bi, Dandan <dandan.bi@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.dong@intel.com>
> 发送时间: 2023年7月31日 13:04
> 收件人: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
> 抄送: Gao, Liming <gaoliming@byosoft.com.cn>
> 主题: RE: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via
> GetVariable for EfiVarStore
>
> Reviewed-by: Eric Dong <eric.dong@intel.com>
>
> -----Original Message-----
> From: Bi, Dandan <dandan.bi@intel.com>
> Sent: Monday, July 31, 2023 8:46 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric
> <eric.dong@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 <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@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 (#107512): https://edk2.groups.io/g/devel/message/107512
Mute This Topic: https://groups.io/mt/100520724/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread
* [edk2-devel] 回复: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore
2023-08-03 4:52 ` [edk2-devel] " Dandan Bi
@ 2023-08-03 11:05 ` gaoliming via groups.io
2023-08-04 6:01 ` [edk2-devel] " Ni, Ray
1 sibling, 0 replies; 8+ messages in thread
From: gaoliming via groups.io @ 2023-08-03 11:05 UTC (permalink / raw)
To: 'Bi, Dandan', 'Dong, Eric', devel
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> -----邮件原件-----
> 发件人: Bi, Dandan <dandan.bi@intel.com>
> 发送时间: 2023年8月3日 12:53
> 收件人: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric
> <eric.dong@intel.com>; devel@edk2.groups.io
> 主题: RE: [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 <gaoliming@byosoft.com.cn>
> Sent: Wednesday, August 2, 2023 1:08 PM
> To: Dong, Eric <eric.dong@intel.com>; Bi, Dandan <dandan.bi@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.dong@intel.com>
> > 发送时间: 2023年7月31日 13:04
> > 收件人: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
> > 抄送: Gao, Liming <gaoliming@byosoft.com.cn>
> > 主题: RE: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via
> > GetVariable for EfiVarStore
> >
> > Reviewed-by: Eric Dong <eric.dong@intel.com>
> >
> > -----Original Message-----
> > From: Bi, Dandan <dandan.bi@intel.com>
> > Sent: Monday, July 31, 2023 8:46 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric
> > <eric.dong@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 <gaoliming@byosoft.com.cn>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Signed-off-by: Dandan Bi <dandan.bi@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 (#107532): https://edk2.groups.io/g/devel/message/107532
Mute This Topic: https://groups.io/mt/100523536/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore
2023-08-03 4:52 ` [edk2-devel] " Dandan Bi
2023-08-03 11:05 ` [edk2-devel] 回复: " gaoliming via groups.io
@ 2023-08-04 6:01 ` Ni, Ray
2023-08-04 6:16 ` Ni, Ray
1 sibling, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2023-08-04 6:01 UTC (permalink / raw)
To: devel@edk2.groups.io, Bi, Dandan, Gao, Liming, Dong, Eric,
Rothman, Michael A
+ @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 <gaoliming@byosoft.com.cn>; Dong, Eric
> <eric.dong@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 <gaoliming@byosoft.com.cn>
> Sent: Wednesday, August 2, 2023 1:08 PM
> To: Dong, Eric <eric.dong@intel.com>; Bi, Dandan <dandan.bi@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.dong@intel.com>
> > 发送时间: 2023年7月31日 13:04
> > 收件人: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
> > 抄送: Gao, Liming <gaoliming@byosoft.com.cn>
> > 主题: RE: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via
> > GetVariable for EfiVarStore
> >
> > Reviewed-by: Eric Dong <eric.dong@intel.com>
> >
> > -----Original Message-----
> > From: Bi, Dandan <dandan.bi@intel.com>
> > Sent: Monday, July 31, 2023 8:46 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric
> > <eric.dong@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 <gaoliming@byosoft.com.cn>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Signed-off-by: Dandan Bi <dandan.bi@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 (#107560): https://edk2.groups.io/g/devel/message/107560
Mute This Topic: https://groups.io/mt/100520724/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore
2023-08-04 6:01 ` [edk2-devel] " Ni, Ray
@ 2023-08-04 6:16 ` Ni, Ray
2023-08-04 7:17 ` Dandan Bi
0 siblings, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2023-08-04 6:16 UTC (permalink / raw)
To: devel@edk2.groups.io, Bi, Dandan, Gao, Liming, Dong, Eric,
Rothman, Michael A
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.bi@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Rothman,
> Michael A <michael.a.rothman@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 <gaoliming@byosoft.com.cn>; Dong, Eric
> > <eric.dong@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 <gaoliming@byosoft.com.cn>
> > Sent: Wednesday, August 2, 2023 1:08 PM
> > To: Dong, Eric <eric.dong@intel.com>; Bi, Dandan <dandan.bi@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.dong@intel.com>
> > > 发送时间: 2023年7月31日 13:04
> > > 收件人: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
> > > 抄送: Gao, Liming <gaoliming@byosoft.com.cn>
> > > 主题: RE: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via
> > > GetVariable for EfiVarStore
> > >
> > > Reviewed-by: Eric Dong <eric.dong@intel.com>
> > >
> > > -----Original Message-----
> > > From: Bi, Dandan <dandan.bi@intel.com>
> > > Sent: Monday, July 31, 2023 8:46 AM
> > > To: devel@edk2.groups.io
> > > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric
> > > <eric.dong@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 <gaoliming@byosoft.com.cn>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Signed-off-by: Dandan Bi <dandan.bi@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore
2023-08-04 6:16 ` Ni, Ray
@ 2023-08-04 7:17 ` Dandan Bi
0 siblings, 0 replies; 8+ messages in thread
From: Dandan Bi @ 2023-08-04 7:17 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io, Gao, Liming, Dong, Eric,
Rothman, Michael A
Cc: Bi, Dandan
Hi Ray,
For varstore of EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER, when SetupBrowser load the value from variable, it will not call into HiiConfigAccess protocol.
Before this change, it will call HiiConfigRouting protocol in HiiDatabase, and HiiDatabase will also call GetVariable directly, does not route to HII driver.
So with this change, it will not cause any flexibility lost in HiiConfigAccess implementation within each HII Driver.
Before this change:
LoadStorage (called by SetupBrowserDxe) ->ExtractConfig (called by SetupBrowserDxe) ->GetVariable(called by HiiDatabase) ->HiiBlockToConfig(called by HiiDatabase) ->ConfigToBlock (called by SetupBrowserDxe)
With this change:
LoadStorage (called by SetupBrowserDxe) ->GetVariable (called by SetupBrowserDxe)
Thanks,
Dandan
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, August 4, 2023 2:17 PM
To: devel@edk2.groups.io; Bi, Dandan <dandan.bi@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Rothman, Michael A <michael.a.rothman@intel.com>
Subject: RE: [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore
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.bi@intel.com>; Gao,
> Liming <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>;
> Rothman, Michael A <michael.a.rothman@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 <gaoliming@byosoft.com.cn>; Dong, Eric
> > <eric.dong@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 <gaoliming@byosoft.com.cn>
> > Sent: Wednesday, August 2, 2023 1:08 PM
> > To: Dong, Eric <eric.dong@intel.com>; Bi, Dandan
> > <dandan.bi@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.dong@intel.com>
> > > 发送时间: 2023年7月31日 13:04
> > > 收件人: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
> > > 抄送: Gao, Liming <gaoliming@byosoft.com.cn>
> > > 主题: RE: [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via
> > > GetVariable for EfiVarStore
> > >
> > > Reviewed-by: Eric Dong <eric.dong@intel.com>
> > >
> > > -----Original Message-----
> > > From: Bi, Dandan <dandan.bi@intel.com>
> > > Sent: Monday, July 31, 2023 8:46 AM
> > > To: devel@edk2.groups.io
> > > Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric
> > > <eric.dong@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 <gaoliming@byosoft.com.cn>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Signed-off-by: Dandan Bi <dandan.bi@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 (#107563): https://edk2.groups.io/g/devel/message/107563
Mute This Topic: https://groups.io/mt/100520724/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-08-04 7:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-31 0:46 [edk2-devel] [PATCH v2] MdeModulePkg/SetupBrowser: Load storage via GetVariable for EfiVarStore Dandan Bi
2023-07-31 5:03 ` Dong, Eric
2023-08-02 5:07 ` [edk2-devel] 回复: " gaoliming via groups.io
2023-08-03 4:52 ` [edk2-devel] " Dandan Bi
2023-08-03 11:05 ` [edk2-devel] 回复: " gaoliming via groups.io
2023-08-04 6:01 ` [edk2-devel] " Ni, Ray
2023-08-04 6:16 ` Ni, Ray
2023-08-04 7:17 ` Dandan Bi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox