Re: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling

["Ni, Ray" <ray.ni@intel.com>]

Star,
It has been merged last week.

> -----Original Message-----
> From: Zeng, Star <star.zeng@intel.com>
> Sent: Tuesday, December 13, 2022 4:41 PM
> To: devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Guenzel, Robert
> <robert.guenzel@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
> 
> Hi,
> 
> When could the patch be merged ?
> 
> Thanks,
> Star
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Jiaxin
> Sent: Wednesday, November 30, 2022 8:47 AM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Guenzel, Robert <robert.guenzel@intel.com>
> Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
> 
> Glad to see this fix, could you add/include the existing Bugzilla in the comment?
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4168
> 
> Thanks,
> Jiaxin
> 
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> > Sent: Wednesday, November 16, 2022 8:57 AM
> > To: devel@edk2.groups.io; Guenzel, Robert <robert.guenzel@intel.com>
> > Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage
> > handling
> >
> > Reviewed-by: Ray Ni <ray.ni@intel.com>
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > Guenzel, Robert
> > > Sent: Thursday, November 10, 2022 9:51 PM
> > > To: devel@edk2.groups.io
> > > Subject: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
> > >
> > > When build in DEBUG, the code asserts that 5LPage support is there
> > > when the physical address width is larger than 48.
> > > In a RELEASE build it will just force LA57 to 1 in CR4 even if
> > > CPUID(7).ECX[16] says it is not supported.
> > >
> > > The hang (in the ASSERT) in DEBUG is not warranted as there are
> > > legal configurations with CPUID(7).ECX[16](==LA57)=0 and with a
> > > physical address width of larger than 48 (like 52).
> > >
> > > This is also supported by this code:
> > >
> > https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/PiSmmCpuDx
> > eSmm/X64/PageTbl.c#L221
> > > There (as long as physical address width is smaller or equal to 52)
> > > any address width above 48 will be reduced to 48 and the system can
> > > and will work without 5LPaging.
> > >
> > > The forced setting of LA57 in CR4 (in the absence of LA57 in
> > > CPUID(7).ECX) is a spec violation and should not happen.
> > >
> > > Hence the proposed fix
> > > a) removes the assert.
> > > b) only returns TRUE from Is5LevelPagingNeeded if 5LPaging is actually
> > >    supported by HW.
> > >
> > > Signed-off-by: Robert Guenzel mailto:robert.guenzel@intel.com
> > > ---
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > index 6587212f4e..f8b1ac31f1 100644
> > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > @@ -104,8 +104,8 @@ Is5LevelPagingNeeded (
> > >      ExtFeatureEcx.Bits.FiveLevelPage
> > >      ));
> > >
> > > -  if (VirPhyAddressSize.Bits.PhysicalAddressBits > 4 * 9 + 12) {
> > > -    ASSERT (ExtFeatureEcx.Bits.FiveLevelPage == 1);
> > > +  if ((VirPhyAddressSize.Bits.PhysicalAddressBits > 4 * 9 + 12) &&
> > > +      (ExtFeatureEcx.Bits.FiveLevelPage == 1)) {
> > >      return TRUE;
> > >    } else {
> > >      return FALSE;
> > > --
> > > 2.34.1
> > > Intel Deutschland GmbH
> > > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> > > Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing
> > > Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
> > > Chairperson of the Supervisory Board: Nicole Lau Registered Office:
> > > Munich Commercial Register: Amtsgericht Muenchen HRB 186928
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
>