Re: [PATCH 1/1] MdeModulePkg/DxeIplPeim: Relocate the operation of PageMapLevel5Entry++

["Dong, Eric" <eric.dong@intel.com>]

Agree with Dandan's comments. With that update, Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Tuesday, August 13, 2019 10:04 AM
> To: Zhang, Shenglei <shenglei.zhang@intel.com>; devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> Subject: RE: [PATCH 1/1] MdeModulePkg/DxeIplPeim: Relocate the
> operation of PageMapLevel5Entry++
> 
> > -----Original Message-----
> > From: Zhang, Shenglei
> > Sent: Monday, August 12, 2019 2:25 PM
> > To: devel@edk2.groups.io
> > Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming
> > <liming.gao@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Laszlo Ersek
> > <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> > Subject: [PATCH 1/1] MdeModulePkg/DxeIplPeim: Relocate the operation
> > of
> > PageMapLevel5Entry++
> >
> > PageMapLevel5Entry may be uninitialized in original code, which means
> > uninitialized pointer will be modified at some circumstance.
> > So relocate the operation of PageMapLevel5Entry++ in order to make
> > sure the pointer could be modified only when it is uninitialized.
> I think "uninitialized" here should be a typo, you may mean "initialized".
> Please update it before commit.
> I have no other comments for this patch.
> Reviewed-by: Dandan Bi <dandan.bi@intel.com>
> 
> >
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
> > ---
> >  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> > b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> > index b40b7e0c9813..2389f3eb485b 100644
> > --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> > +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> > @@ -764,7 +764,7 @@ CreateIdentityMappingPageTables (
> >
> >    for ( IndexOfPml5Entries = 0
> >        ; IndexOfPml5Entries < NumberOfPml5EntriesNeeded
> > -      ; IndexOfPml5Entries++, PageMapLevel5Entry++) {
> > +      ; IndexOfPml5Entries++) {
> >      //
> >      // Each PML5 entry points to a page of PML4 entires.
> >      // So lets allocate space for them and fill them in in the
> > IndexOfPml4Entries loop.
> > @@ -780,6 +780,7 @@ CreateIdentityMappingPageTables (
> >        PageMapLevel5Entry->Uint64 = (UINT64) (UINTN)
> > PageMapLevel4Entry | AddressEncMask;
> >        PageMapLevel5Entry->Bits.ReadWrite = 1;
> >        PageMapLevel5Entry->Bits.Present   = 1;
> > +      PageMapLevel5Entry++;
> >      }
> >
> >      for ( IndexOfPml4Entries = 0
> > --
> > 2.18.0.windows.1