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...@intel.com>
> Sent: Tuesday, January 17, 2023 8:49 PM
> To: devel@edk2.groups.io; kra...@redhat.com
> Cc: Liu, Zhiguang <zhiguang....@intel.com>; Kumar, Rahul R
> <rahul.r.ku...@intel.com>; Dong, Eric <eric.d...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
