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: kra...@redhat.com <kra...@redhat.com> > Sent: Wednesday, January 18, 2023 4:54 PM > To: devel@edk2.groups.io; Liu, Zhiguang <zhiguang....@intel.com> > Cc: Ni, Ray <ray...@intel.com>; Kumar, Rahul R <rahul.r.ku...@intel.com>; > Dong, Eric <eric.d...@intel.com>; Zeng, Star <star.z...@intel.com>; Wu, > Jiaxin <jiaxin...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-