1. Patchew
  2. EDK2
  3. View series

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

Guenzel, Robert posted 1 patch 1 year, 5 months ago
Failed in applying to current master (apply log)
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
Posted by Guenzel, Robert 1 year, 5 months ago 
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/PiSmmCpuDxeSmm/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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96192): https://edk2.groups.io/g/devel/message/96192
Mute This Topic: https://groups.io/mt/94935896/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
Posted by Ni, Ray 1 year, 5 months ago 
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/PiSmmCpuDxeSmm/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
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96410): https://edk2.groups.io/g/devel/message/96410
Mute This Topic: https://groups.io/mt/94935896/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
Posted by Wu, Jiaxin 1 year, 4 months ago 
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
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96720): https://edk2.groups.io/g/devel/message/96720
Mute This Topic: https://groups.io/mt/94935896/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
Posted by Zeng, Star 1 year, 4 months ago 
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
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97300): https://edk2.groups.io/g/devel/message/97300
Mute This Topic: https://groups.io/mt/94935896/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
Posted by Ni, Ray 1 year, 4 months ago 
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
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97301): https://edk2.groups.io/g/devel/message/97301
Mute This Topic: https://groups.io/mt/94935896/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
Posted by Zeng, Star 1 year, 4 months ago 
Thanks Ray. 😊

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Tuesday, December 13, 2022 5:05 PM
To: Zeng, Star <star.zeng@intel.com>; devel@edk2.groups.io; Wu, Jiaxin <jiaxin.wu@intel.com>; Guenzel, Robert <robert.guenzel@intel.com>
Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling

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
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97305): https://edk2.groups.io/g/devel/message/97305
Mute This Topic: https://groups.io/mt/94935896/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] UefiCpuPkg: Bug fix in 5LPage handling
Posted by Michael D Kinney 1 year, 5 months ago 
+UefiCpuPkg maintainers/reviewers.


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guenzel, Robert
> Sent: Thursday, November 10, 2022 5:51 AM
> 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/PiSmmCpuDxeSmm/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
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#96408): https://edk2.groups.io/g/devel/message/96408
Mute This Topic: https://groups.io/mt/94935896/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.