Re: [edk2-devel] [RFC edk2 v1 1/1] MdeModulePkg/Variable: Move FindVariable after AutoUpdateLangVariable

["Guomin Jiang" <guomin.jiang@intel.com>]

Hi Huangming,

I will double check it when I am free.

> -----Original Message-----
> From: Ming Huang <huangming23@huawei.com>
> Sent: Wednesday, May 27, 2020 10:40 PM
> To: Jiang, Guomin <guomin.jiang@intel.com>; devel@edk2.groups.io; Wang,
> Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao,
> Liming <liming.gao@intel.com>
> Cc: lidongzhan@huawei.com; songdongkuang@huawei.com;
> wanghuiqiang@huawei.com; qiuliangen@huawei.com;
> shenlimei@huawei.com
> Subject: Re: [edk2-devel] [RFC edk2 v1 1/1] MdeModulePkg/Variable: Move
> FindVariable after AutoUpdateLangVariable
> 
> Hi Guomin,
> 
> This patch change the position of invoking the FindVariable() function to after
> AutoUpdateLangVariable() invoked.
> 
> Thanks,
> Ming
> 
> 在 2020/5/27 8:12, Jiang, Guomin 写道:
> > Hi Huangming,
> >
> > I will clarify it when I am free, please be patient.
> >
> > If it is urgent for you, I suggest that you can use your patch temporarily.
> >
> > Any other thing, I check your patch in the mail, it seem that not any change
> in code, can you double confirm it?
> >
> > Best Regards
> >> -----Original Message-----
> >> From: Ming Huang <huangming23@huawei.com>
> >> Sent: Tuesday, May 26, 2020 2:08 PM
> >> To: Jiang, Guomin <guomin.jiang@intel.com>; devel@edk2.groups.io;
> >> Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> >> Gao, Liming <liming.gao@intel.com>
> >> Cc: lidongzhan@huawei.com; songdongkuang@huawei.com;
> >> wanghuiqiang@huawei.com; qiuliangen@huawei.com;
> shenlimei@huawei.com
> >> Subject: Re: [edk2-devel] [RFC edk2 v1 1/1] MdeModulePkg/Variable:
> >> Move FindVariable after AutoUpdateLangVariable
> >>
> >>
> >>
> >> 在 2020/5/26 8:39, Jiang, Guomin 写道:
> >>> Hi Huangming,
> >>>
> >>> I am taking the bugzilla and I am sorry that I haven't provide you
> >>> with
> >> productive comment.
> >>>
> >>> I am still busy until August.
> >>>
> >>> I just want to know that:
> >>> 1. Have you verified that the symptom will disappear after invoked
> >> FindVariable() function?
> >>
> >> Yes, the symptom will disappeare after add this patch.
> >>
> >>> 2. Is it your suggestion that the FindVariable() need to be invoked
> >>> but you
> >> have no idea that how to fix it?
> >>
> >> This patch can fix this issue, and I guess this issue was resulted by
> >> adding AutoUpdateLangVariable feature.
> >> I hope this patch can be merged to edk2 master.
> >>
> >> Thanks
> >> Ming
> >>
> >>>
> >>> Best Regards
> >>> Guomin
> >>>> -----Original Message-----
> >>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Ming
> >>>> Huang
> >>>> Sent: Monday, May 25, 2020 7:34 PM
> >>>> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu,
> >>>> Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>
> >>>> Cc: lidongzhan@huawei.com; huangming23@huawei.com;
> >>>> songdongkuang@huawei.com; wanghuiqiang@huawei.com;
> >>>> qiuliangen@huawei.com; shenlimei@huawei.com
> >>>> Subject: [edk2-devel] [RFC 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 <huangming23@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
> >>>>
> >>>>
> >>>> 
> >>>
> >>>
> >>>
> >
> >
> > .
> >