Hi Ming,
The new posted change https://edk2.groups.io/g/devel/topic/75412007#62327 may
be helpful for this issue.
Can you add the change in your code and verify it?
Thanks
Guomin
> -----Original Message-----
> From: Ming Huang <huangmin...@huawei.com>
> Sent: Friday, July 3, 2020 8:49 PM
> To: Jiang, Guomin <guomin.ji...@intel.com>; devel@edk2.groups.io; Wang,
> Jian J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; Gao,
> Liming <liming....@intel.com>
> Cc: lidongz...@huawei.com; songdongku...@huawei.com;
> wanghuiqi...@huawei.com; qiulian...@huawei.com;
> shenli...@huawei.com; xiewen...@huawei.com; john.ga...@huawei.com
> Subject: Re: [edk2-devel] [PATCH edk2 v1 1/1] MdeModulePkg/Variable:
> Move FindVariable after AutoUpdateLangVariable
>
>
>
> 在 2020/7/1 8:22, Jiang, Guomin 写道:
> > So I think the key point is why AutoUpdateLangVariable() return success
> rather than fail, if is it reasonable for this case or we need other error
> handing?
>
> I don't think AutoUpdateLangVariable() should return fail while occur reclaim
> internal in AutoUpdateLangVariable () function. The problem is the
> Variable(VARIABLE_POINTER_TRACK) get by FindVariable is invald in this
> situation and this Variable will be pass to UpdateVariable().
>
> if (mVariableModuleGlobal->VariableGlobal.AuthSupport) {
> Status = AuthVariableLibProcessVariable (VariableName, VendorGuid,
> Data, DataSize, Attributes);
> } else {
> // This Variable is invald while occur reclaim internal in
> AutoUpdateLangVariable ()
> Status = UpdateVariable (VariableName, VendorGuid, Data, DataSize,
> Attributes, 0, 0, &Variable, NULL);
> }
>
> >
> > I am glad to help you but I can't reproduce it until now, can you provide a
> step to reproduce it in simulation platform.
>
> I am not familiar with simulation platform. We reproduct this issue in our
> board once.
> For accelerating reproduction this issue, Add Reclaim() to
> AutoUpdateLangVariable() for test.
>
> Thanks,
> Ming
>
> >
> > If it is urgent, I suggest that discuss with your internal team first and
> > explain
> that we need consider the risk check it into edk2.
> >
> > Best Regards
> > Guomin
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ming
> >> Huang via groups.io
> >> Sent: Tuesday, June 30, 2020 8:26 PM
> >> To: Jiang, Guomin <guomin.ji...@intel.com>; devel@edk2.groups.io;
> >> Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>;
> >> Gao, Liming <liming....@intel.com>
> >> Cc: lidongz...@huawei.com; songdongku...@huawei.com;
> >> wanghuiqi...@huawei.com; qiulian...@huawei.com;
> shenli...@huawei.com;
> >> xiewen...@huawei.com
> >> Subject: Re: [edk2-devel] [PATCH edk2 v1 1/1] MdeModulePkg/Variable:
> >> Move FindVariable after AutoUpdateLangVariable
> >>
> >>
> >>
> >> 在 2020/6/30 8:58, Jiang, Guomin 写道:
> >>> Hi Huang,
> >>>
> >>> >From issue statement, I guess that
> >>> 1. AutoUpdateLangVariable() invoked, and it will invoke
> >>> FindVariable() first, at the same time, reclaim occur and
> >>> Variable.CurrPtr is invalid, it return
> >> with success 2. UpdateVariable() is invoked when The old Lang's State
> >> is valid and the new Lang's State is also valid.
> >>> 3. In the situation, FindVariable() checked Lang's State and only
> >>> enable one
> >> Lang's State. But it didn't in fact.
> >>> 4. BmForEachVariable() deadloop in the situation.
> >>>
> >>> Am I right?
> >>
> >> Yes, right.
> >>
> >> Thanks,
> >> Ming
> >>
> >>>
> >>>> -----Original Message-----
> >>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Ming
> >>>> Huang via groups.io
> >>>> Sent: Monday, June 29, 2020 2:06 PM
> >>>> To: devel@edk2.groups.io; Wang, Jian J <jian.j.w...@intel.com>; Wu,
> >>>> Hao A <hao.a...@intel.com>; Gao, Liming <liming....@intel.com>
> >>>> Cc: lidongz...@huawei.com; huangmin...@huawei.com;
> >>>> songdongku...@huawei.com; wanghuiqi...@huawei.com;
> >>>> qiulian...@huawei.com; shenli...@huawei.com;
> >> xiewen...@huawei.com
> >>>> Subject: [edk2-devel] [PATCH edk2 v1 1/1] MdeModulePkg/Variable:
> >> Move
> >>>> FindVariable after AutoUpdateLangVariable
> >>>>
> >>>> When occur reclaim in AutoUpdateLangVariable(), the CurrPtr of
> >>>> Variable is invalid. The State will be update with wrong position
> >>>> after UpdateVariable in this situation and two valid PlatformLang
> >>>> or Lang
> >> variables will exist.
> >>>> BmForEachVariable() will enter endless loop while exist two valid
> >>>> PlatformLang variables. So FindVariable() should be invoked atfer
> >>>> AutoUpdateLangVariable().
> >>>>
> >>>> https://bugzilla.tianocore.org/show_bug.cgi?id=2667
> >>>>
> >>>> Signed-off-by: Ming Huang <huangmin...@huawei.com>
> >>>> ---
> >>>> MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 26
> >>>> ++++++++++----------
> >>>> 1 file changed, 13 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>>> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>>> index 1e71fc6..0cec981 100644
> >>>> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>>> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> >>>> @@ -2741,6 +2741,19 @@ VariableServiceSetVariable (
> >>>> mVariableModuleGlobal->NonVolatileLastVariableOffset = (UINTN)
> >>>> NextVariable - (UINTN) Point;
> >>>> }
> >>>>
> >>>> + if (!FeaturePcdGet (PcdUefiVariableDefaultLangDeprecate)) {
> >>>> + //
> >>>> + // Hook the operation of setting
> >>>> + PlatformLangCodes/PlatformLang and
> >>>> LangCodes/Lang.
> >>>> + //
> >>>> + Status = AutoUpdateLangVariable (VariableName, Data, DataSize);
> >>>> + if (EFI_ERROR (Status)) {
> >>>> + //
> >>>> + // The auto update operation failed, directly return to
> >>>> + avoid
> >>>> inconsistency between PlatformLang and Lang.
> >>>> + //
> >>>> + goto Done;
> >>>> + }
> >>>> + }
> >>>> +
> >>>> //
> >>>> // Check whether the input variable is already existed.
> >>>> //
> >>>> @@ -2763,19 +2776,6 @@ VariableServiceSetVariable (
> >>>> }
> >>>> }
> >>>>
> >>>> - if (!FeaturePcdGet (PcdUefiVariableDefaultLangDeprecate)) {
> >>>> - //
> >>>> - // Hook the operation of setting PlatformLangCodes/PlatformLang
> and
> >>>> LangCodes/Lang.
> >>>> - //
> >>>> - Status = AutoUpdateLangVariable (VariableName, Data, DataSize);
> >>>> - if (EFI_ERROR (Status)) {
> >>>> - //
> >>>> - // The auto update operation failed, directly return to avoid
> >> inconsistency
> >>>> between PlatformLang and Lang.
> >>>> - //
> >>>> - goto Done;
> >>>> - }
> >>>> - }
> >>>> -
> >>>> if (mVariableModuleGlobal->VariableGlobal.AuthSupport) {
> >>>> Status = AuthVariableLibProcessVariable (VariableName,
> >>>> VendorGuid, Data, DataSize, Attributes);
> >>>> } else {
> >>>> --
> >>>> 2.8.1
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>
> >>
> >>
> >
> >
> > .
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#63032): https://edk2.groups.io/g/devel/message/63032
Mute This Topic: https://groups.io/mt/75186349/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
