UefiCpuPkg: Fix SMM code hangs when InitPaging

[edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Zhiguang Liu 1 year, 4 months ago

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4246

In function InitPaging, NumberOfPml5Entries is calculated by below code
NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
If the SizeOfMemorySpace is larger than 48, NumberOfPml5Entries will be
larger than 1. However, this doesn't make sense if the hardware doesn't
support 5 level page table.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index c1efda7126..c597b39b8c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -1,7 +1,7 @@
 /** @file
 Enable SMM profile.
 
-Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2023, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -587,15 +587,17 @@ InitPaging (
     }
 
     SizeOfMemorySpace = HighBitSet64 (gPhyMask) + 1;
+    ASSERT (SizeOfMemorySpace <= 52);
+
     //
-    // Calculate the table entries of PML4E and PDPTE.
+    // Calculate the table entries of PML5E, PML4E and PDPTE.
     //
     NumberOfPml5Entries = 1;
-    if (SizeOfMemorySpace > 48) {
+    if (Enable5LevelPaging && (SizeOfMemorySpace > 48)) {
       NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
-      SizeOfMemorySpace   = 48;
     }
 
+    SizeOfMemorySpace   = SizeOfMemorySpace > 48 ? 48 : SizeOfMemorySpace;
     NumberOfPml4Entries = 1;
     if (SizeOfMemorySpace > 39) {
       NumberOfPml4Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 39);
-- 
2.31.1.windows.1



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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Ni, Ray 1 year, 3 months ago

+ Gerd.

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Wednesday, January 4, 2023 1:41 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Dong,
> Eric <eric.dong@intel.com>
> Subject: [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4246
> 
> In function InitPaging, NumberOfPml5Entries is calculated by below code
> NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
> If the SizeOfMemorySpace is larger than 48, NumberOfPml5Entries will be
> larger than 1. However, this doesn't make sense if the hardware doesn't
> support 5 level page table.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index c1efda7126..c597b39b8c 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -1,7 +1,7 @@
>  /** @file
>  Enable SMM profile.
> 
> -Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 2023, Intel Corporation. All rights reserved.<BR>
>  Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -587,15 +587,17 @@ InitPaging (
>      }
> 
>      SizeOfMemorySpace = HighBitSet64 (gPhyMask) + 1;
> +    ASSERT (SizeOfMemorySpace <= 52);
> +
>      //
> -    // Calculate the table entries of PML4E and PDPTE.
> +    // Calculate the table entries of PML5E, PML4E and PDPTE.
>      //
>      NumberOfPml5Entries = 1;
> -    if (SizeOfMemorySpace > 48) {
> +    if (Enable5LevelPaging && (SizeOfMemorySpace > 48)) {
>        NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
> -      SizeOfMemorySpace   = 48;
>      }
> 
> +    SizeOfMemorySpace   = SizeOfMemorySpace > 48 ? 48 : SizeOfMemorySpace;
>      NumberOfPml4Entries = 1;
>      if (SizeOfMemorySpace > 39) {
>        NumberOfPml4Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 39);
> --
> 2.31.1.windows.1



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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Gerd Hoffmann 1 year, 3 months ago

On Tue, Jan 17, 2023 at 09:02:01AM +0000, Ni, Ray wrote:
> + Gerd.
> 
> > -----Original Message-----
> > From: Liu, Zhiguang <zhiguang.liu@intel.com>
> > Sent: Wednesday, January 4, 2023 1:41 PM
> > To: devel@edk2.groups.io
> > Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Dong,
> > Eric <eric.dong@intel.com>
> > Subject: [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging
> > 
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4246
> > 
> > In function InitPaging, NumberOfPml5Entries is calculated by below code
> > NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
> > If the SizeOfMemorySpace is larger than 48, NumberOfPml5Entries will be
> > larger than 1. However, this doesn't make sense if the hardware doesn't
> > support 5 level page table.

> > +    ASSERT (SizeOfMemorySpace <= 52);
> > +
> >      //
> > -    // Calculate the table entries of PML4E and PDPTE.
> > +    // Calculate the table entries of PML5E, PML4E and PDPTE.
> >      //
> >      NumberOfPml5Entries = 1;
> > -    if (SizeOfMemorySpace > 48) {
> > +    if (Enable5LevelPaging && (SizeOfMemorySpace > 48)) {
> >        NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
> > -      SizeOfMemorySpace   = 48;
> >      }
> > 
> > +    SizeOfMemorySpace   = SizeOfMemorySpace > 48 ? 48 : SizeOfMemorySpace;

if (SizeOfMemorySpace > 48) {
    if (Enable5LevelPaging) {
         NumberOfPml5Entries = ...
    }
    SizeOfMemorySpace = 48
}

That is a much more readable version.

The only effect I can see is that this avoids creating page tables which
would not be used anyway.

Can you explain where the hangs mentioned in the subject line are coming
from and why the patch fixes them?

take care,
  Gerd



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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Ni, Ray 1 year, 3 months ago

> > > +    ASSERT (SizeOfMemorySpace <= 52);
> > > +
> > >      //
> > > -    // Calculate the table entries of PML4E and PDPTE.
> > > +    // Calculate the table entries of PML5E, PML4E and PDPTE.
> > >      //
> > >      NumberOfPml5Entries = 1;
> > > -    if (SizeOfMemorySpace > 48) {
> > > +    if (Enable5LevelPaging && (SizeOfMemorySpace > 48)) {
> > >        NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
> > > -      SizeOfMemorySpace   = 48;
> > >      }
> > >
> > > +    SizeOfMemorySpace   = SizeOfMemorySpace > 48 ? 48 : SizeOfMemorySpace;
> 
> if (SizeOfMemorySpace > 48) {
>     if (Enable5LevelPaging) {
>          NumberOfPml5Entries = ...
>     }
>     SizeOfMemorySpace = 48
> }
> 
> That is a much more readable version.

I had the same thought. New version is consistent with the logic below.


> 
> The only effect I can see is that this avoids creating page tables which
> would not be used anyway.
> 
> Can you explain where the hangs mentioned in the subject line are coming
> from and why the patch fixes them?


> 
> take care,
>   Gerd
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Zhiguang Liu 1 year, 3 months ago

Thanks all for reviewing, and I will send a new version to address the comment.

As for Gerd's question, let me explain.
Let's see one example, that the CPU has SizeOfMemorySpace >48, but the CPU doesn't enable 5 level paging.
The purpose of the current function InitPaging is to modify existing page table. To use the same logic to handle both 5 level and 4 level paging, for 4 level paging, the logic will create a false 5 level paging entry to treat it like a 5 level page table. This way, the number of 5 level paging should always be one. If we use SizeOfMemorySpace to calculate the 5 level paging entry count, we will get number more than one.  However, as I just mentioned, we only create one false 5 level paging entry, system may hang when we try to access the second 5 level paging entry. This patch fixes the issue by always letting the number of 5 level paging entry as one if 5 level paging is disabled.

Thanks
Zhiguang

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Tuesday, January 17, 2023 8:49 PM
> To: devel@edk2.groups.io; kraxel@redhat.com
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Kumar, Rahul R
> <rahul.r.kumar@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when
> InitPaging
> 
> > > > +    ASSERT (SizeOfMemorySpace <= 52);
> > > > +
> > > >      //
> > > > -    // Calculate the table entries of PML4E and PDPTE.
> > > > +    // Calculate the table entries of PML5E, PML4E and PDPTE.
> > > >      //
> > > >      NumberOfPml5Entries = 1;
> > > > -    if (SizeOfMemorySpace > 48) {
> > > > +    if (Enable5LevelPaging && (SizeOfMemorySpace > 48)) {
> > > >        NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace -
> 48);
> > > > -      SizeOfMemorySpace   = 48;
> > > >      }
> > > >
> > > > +    SizeOfMemorySpace   = SizeOfMemorySpace > 48 ? 48 :
> SizeOfMemorySpace;
> >
> > if (SizeOfMemorySpace > 48) {
> >     if (Enable5LevelPaging) {
> >          NumberOfPml5Entries = ...
> >     }
> >     SizeOfMemorySpace = 48
> > }
> >
> > That is a much more readable version.
> 
> I had the same thought. New version is consistent with the logic below.
> 
> 
> >
> > The only effect I can see is that this avoids creating page tables
> > which would not be used anyway.
> >
> > Can you explain where the hangs mentioned in the subject line are
> > coming from and why the patch fixes them?
> 
> 
> >
> > take care,
> >   Gerd
> >
> >
> >
> > 
> >



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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Gerd Hoffmann 1 year, 3 months ago

On Wed, Jan 18, 2023 at 01:13:43AM +0000, Zhiguang Liu wrote:
> Thanks all for reviewing, and I will send a new version to address the comment.
> 
> As for Gerd's question, let me explain.
> Let's see one example, that the CPU has SizeOfMemorySpace >48, but the CPU doesn't enable 5 level paging.
> The purpose of the current function InitPaging is to modify existing
> page table. To use the same logic to handle both 5 level and 4 level
> paging, for 4 level paging, the logic will create a false 5 level
> paging entry to treat it like a 5 level page table.

Yes.  Same for 3-level paging btw.  There are always page tables for 5
levels, but the higher levels might be unused.

> This way, the
> number of 5 level paging should always be one. If we use
> SizeOfMemorySpace to calculate the 5 level paging entry count, we will
> get number more than one.  However, as I just mentioned, we only
> create one false 5 level paging entry, system may hang when we try to
> access the second 5 level paging entry.

If 5-level paging is turned off the CPU should not see what you are
doing with the page tables for the second (and higher) 5-level entry.

So, limiting the number of 5-level entries does make sense.  Higher
entries are not used, so it's pointless work.

But that doesn't answer the question:  Why does that fix the system
hanging?  I just can't see a reason for that when looking through the
InitPaging code.  I suspect this might hide a bug somewhere else.

Related:  We got UefiCpuPkg/Library/CpuPageTableLib last year, can this
be used instead?

take care,
  Gerd

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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Zhiguang Liu 1 year, 3 months ago

Hi Gerd,

Let's check the code in InitPaging.
If 5LevelPaging is disabled, Pml5 points to a local variable. Pml5[1] shouldn't be used.

     UINT64    Pml5Entry;
     UINT64    *Pml5;
     if (!Enable5LevelPaging) {
        Pml5Entry = (UINTN)mSmmProfileCr3 | IA32_PG_P;
        Pml5      = &Pml5Entry;

However, if NumberOfPml5Entries is larger than 1, below code will access Pml5[1], which may cause unexpected future code flow.

    for (Pml5Index = 0; Pml5Index < NumberOfPml5Entries; Pml5Index++) {
        if ((Pml5[Pml5Index] & IA32_PG_P) == 0) {

Could this can answer your question? Please let me know if you still have concern.

And for the CpuPageTableLib, I think the API don't provide the interface to split 2MB-page page table into 4KB-page, which is the function wants to do.

Thanks
Zhiguang


> -----Original Message-----
> From: kraxel@redhat.com <kraxel@redhat.com>
> Sent: Wednesday, January 18, 2023 4:54 PM
> To: devel@edk2.groups.io; Liu, Zhiguang <zhiguang.liu@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when
> InitPaging
> 
> On Wed, Jan 18, 2023 at 01:13:43AM +0000, Zhiguang Liu wrote:
> > Thanks all for reviewing, and I will send a new version to address the comment.
> >
> > As for Gerd's question, let me explain.
> > Let's see one example, that the CPU has SizeOfMemorySpace >48, but the CPU
> doesn't enable 5 level paging.
> > The purpose of the current function InitPaging is to modify existing
> > page table. To use the same logic to handle both 5 level and 4 level
> > paging, for 4 level paging, the logic will create a false 5 level
> > paging entry to treat it like a 5 level page table.
> 
> Yes.  Same for 3-level paging btw.  There are always page tables for 5 levels, but
> the higher levels might be unused.
> 
> > This way, the
> > number of 5 level paging should always be one. If we use
> > SizeOfMemorySpace to calculate the 5 level paging entry count, we will
> > get number more than one.  However, as I just mentioned, we only
> > create one false 5 level paging entry, system may hang when we try to
> > access the second 5 level paging entry.
> 
> If 5-level paging is turned off the CPU should not see what you are doing with
> the page tables for the second (and higher) 5-level entry.
> 
> So, limiting the number of 5-level entries does make sense.  Higher entries are
> not used, so it's pointless work.
> 
> But that doesn't answer the question:  Why does that fix the system hanging?  I
> just can't see a reason for that when looking through the InitPaging code.  I
> suspect this might hide a bug somewhere else.
> 
> Related:  We got UefiCpuPkg/Library/CpuPageTableLib last year, can this be
> used instead?
> 
> take care,
>   Gerd



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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Gerd Hoffmann 1 year, 3 months ago

On Wed, Jan 18, 2023 at 09:12:09AM +0000, Zhiguang Liu wrote:
> Hi Gerd,
> 
> Let's check the code in InitPaging.
> If 5LevelPaging is disabled, Pml5 points to a local variable. Pml5[1] shouldn't be used.
> 
>      UINT64    Pml5Entry;
>      UINT64    *Pml5;
>      if (!Enable5LevelPaging) {
>         Pml5Entry = (UINTN)mSmmProfileCr3 | IA32_PG_P;
>         Pml5      = &Pml5Entry;

Oh, it's just a dummy entry on the stack, not an dummy page table.
Missed that detail.

So writing entry #2 and higher smashes the stack.  That certainly
explains why the code hangs.

> And for the CpuPageTableLib, I think the API don't provide the
> interface to split 2MB-page page table into 4KB-page, which is the
> function wants to do.

I think that is handled by the library automatically.  You can request
address ranges being mapped with specific attributes (such as NX set),
and the library will transparently split pages for you if needed.

take care,
  Gerd



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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Ni, Ray 1 year, 3 months ago

> 
> > And for the CpuPageTableLib, I think the API don't provide the
> > interface to split 2MB-page page table into 4KB-page, which is the
> > function wants to do.
> 
> I think that is handled by the library automatically.  You can request
> address ranges being mapped with specific attributes (such as NX set),
> and the library will transparently split pages for you if needed.
> 

Replacing today's duplicated page table manipulation logic with PageTableLib
is in the todo list (as I said earlier creating a page table library was in the todo
list).
The bug is critical and needs to be fixed asap.
Using lib will be done in future.
That will be great if the someone from the community can help on that part.

Thanks,
Ray


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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Ni, Ray 1 year, 3 months ago

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Wednesday, January 4, 2023 1:41 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Dong,
> Eric <eric.dong@intel.com>
> Subject: [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4246
> 
> In function InitPaging, NumberOfPml5Entries is calculated by below code
> NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
> If the SizeOfMemorySpace is larger than 48, NumberOfPml5Entries will be
> larger than 1. However, this doesn't make sense if the hardware doesn't
> support 5 level page table.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index c1efda7126..c597b39b8c 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -1,7 +1,7 @@
>  /** @file
>  Enable SMM profile.
> 
> -Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 2023, Intel Corporation. All rights reserved.<BR>
>  Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -587,15 +587,17 @@ InitPaging (
>      }
> 
>      SizeOfMemorySpace = HighBitSet64 (gPhyMask) + 1;
> +    ASSERT (SizeOfMemorySpace <= 52);
> +
>      //
> -    // Calculate the table entries of PML4E and PDPTE.
> +    // Calculate the table entries of PML5E, PML4E and PDPTE.
>      //
>      NumberOfPml5Entries = 1;
> -    if (SizeOfMemorySpace > 48) {
> +    if (Enable5LevelPaging && (SizeOfMemorySpace > 48)) {
>        NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
> -      SizeOfMemorySpace   = 48;
>      }
> 
> +    SizeOfMemorySpace   = SizeOfMemorySpace > 48 ? 48 : SizeOfMemorySpace;
>      NumberOfPml4Entries = 1;
>      if (SizeOfMemorySpace > 39) {
>        NumberOfPml4Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 39);
> --
> 2.31.1.windows.1



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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Zeng, Star 1 year, 3 months ago

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhiguang Liu
Sent: Wednesday, January 4, 2023 1:41 PM
To: devel@edk2.groups.io
Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4246

In function InitPaging, NumberOfPml5Entries is calculated by below code NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48); If the SizeOfMemorySpace is larger than 48, NumberOfPml5Entries will be larger than 1. However, this doesn't make sense if the hardware doesn't support 5 level page table.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index c1efda7126..c597b39b8c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -1,7 +1,7 @@
 /** @file
 Enable SMM profile.
 
-Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2012 - 2023, Intel Corporation. All rights reserved.<BR>
 Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
 
 SPDX-License-Identifier: BSD-2-Clause-Patent @@ -587,15 +587,17 @@ InitPaging (
     }
 
     SizeOfMemorySpace = HighBitSet64 (gPhyMask) + 1;
+    ASSERT (SizeOfMemorySpace <= 52);
+
     //
-    // Calculate the table entries of PML4E and PDPTE.
+    // Calculate the table entries of PML5E, PML4E and PDPTE.
     //
     NumberOfPml5Entries = 1;
-    if (SizeOfMemorySpace > 48) {
+    if (Enable5LevelPaging && (SizeOfMemorySpace > 48)) {
       NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
-      SizeOfMemorySpace   = 48;
     }
 
+    SizeOfMemorySpace   = SizeOfMemorySpace > 48 ? 48 : SizeOfMemorySpace;
     NumberOfPml4Entries = 1;
     if (SizeOfMemorySpace > 39) {
       NumberOfPml4Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 39);
--
2.31.1.windows.1








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

Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when InitPaging

Posted by Wu, Jiaxin 1 year, 3 months ago

Reviewed-by: Wu, Jiaxin <jiaxin.wu@intel.com>



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zeng,
> Star
> Sent: Thursday, January 12, 2023 8:12 PM
> To: devel@edk2.groups.io; Liu, Zhiguang <zhiguang.liu@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Tan, Dun <dun.tan@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when
> InitPaging
> 
> Reviewed-by: Star Zeng <star.zeng@intel.com>
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Zhiguang Liu
> Sent: Wednesday, January 4, 2023 1:41 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Kumar, Rahul R <rahul.r.kumar@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2-devel] [PATCH] UefiCpuPkg: Fix SMM code hangs when
> InitPaging
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4246
> 
> In function InitPaging, NumberOfPml5Entries is calculated by below code
> NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48); If
> the SizeOfMemorySpace is larger than 48, NumberOfPml5Entries will be
> larger than 1. However, this doesn't make sense if the hardware doesn't
> support 5 level page table.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index c1efda7126..c597b39b8c 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -1,7 +1,7 @@
>  /** @file
>  Enable SMM profile.
> 
> -Copyright (c) 2012 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2012 - 2023, Intel Corporation. All rights reserved.<BR>
>  Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
> 
>  SPDX-License-Identifier: BSD-2-Clause-Patent @@ -587,15 +587,17 @@
> InitPaging (
>      }
> 
>      SizeOfMemorySpace = HighBitSet64 (gPhyMask) + 1;
> +    ASSERT (SizeOfMemorySpace <= 52);
> +
>      //
> -    // Calculate the table entries of PML4E and PDPTE.
> +    // Calculate the table entries of PML5E, PML4E and PDPTE.
>      //
>      NumberOfPml5Entries = 1;
> -    if (SizeOfMemorySpace > 48) {
> +    if (Enable5LevelPaging && (SizeOfMemorySpace > 48)) {
>        NumberOfPml5Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 48);
> -      SizeOfMemorySpace   = 48;
>      }
> 
> +    SizeOfMemorySpace   = SizeOfMemorySpace > 48 ? 48 :
> SizeOfMemorySpace;
>      NumberOfPml4Entries = 1;
>      if (SizeOfMemorySpace > 39) {
>        NumberOfPml4Entries = (UINTN)LShiftU64 (1, SizeOfMemorySpace - 39);
> --
> 2.31.1.windows.1
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 



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