Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports
On 07/11/19 03:19, Ni, Ray wrote: > Mike, > Thanks for raising this build failure. > I just tried in my Ubuntu 18 in Win10. Even GCC7 complains about this. My bad! > I just posted a fix. Thanks -- as I requested there, please do not push this new patch until the revert+reapply completes. Laszlo >> -Original Message- >> From: Kinney, Michael D >> Sent: Thursday, July 11, 2019 4:06 AM >> To: devel@edk2.groups.io; Ni, Ray ; Kinney, Michael D >> >> Cc: Dong, Eric ; Laszlo Ersek >> Subject: RE: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level >> paging when CPU supports >> >> Hi Ray, >> >> I noticed a Linux/GCC build issue with this patch when using GCC version: >> >> gcc version 8.2.1 20181215 (Red Hat 8.2.1-6) (GCC) >> >> edk2/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c: In function 'ReclaimPages': >> edk2/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c:574:89: error: ?: using integer >> constants in boolean context, the >> expression will always evaluate to 'true' [-Werror=int-in-bool-context] >>for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / >> sizeof (*Pml4)) : 1; Pml5Index++) { >> >> I was able to get the build to pass if I added (). >> >> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> index c31160735a..a3b62f7787 100644 >> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c >> @@ -571,7 +571,7 @@ ReclaimPages ( >>// >>// First, find the leaf entry has the smallest access record value >>// >> - for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / >> sizeof (*Pml4)) : 1; Pml5Index++) { >> + for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / >> sizeof (*Pml4)) : 1); Pml5Index++) {^M >> if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & >> IA32_PG_PMNT) != 0) { >>// >>// If the PML5 entry is not present or is masked, skip it >> >> Best regards, >> >> Mike >> >>> -Original Message- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] >>> On Behalf Of Ni, Ray >>> Sent: Tuesday, July 2, 2019 11:54 PM >>> To: devel@edk2.groups.io >>> Cc: Dong, Eric ; Laszlo Ersek >>> >>> Subject: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: >>> Enable 5 level paging when CPU supports >>> >>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1946 >>> >>> The patch changes SMM environment to use 5 level paging >>> when CPU >>> supports it. >>> >>> Signed-off-by: Ray Ni >>> Cc: Eric Dong >>> Regression-tested-by: Laszlo Ersek >>> --- >>> .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 20 +- >>> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c| 272 >>> ++ >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 485 >>> -- >>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 12 + >>> .../PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 72 ++- >>> 5 files changed, 561 insertions(+), 300 deletions(-) >>> >>> diff --git >>> a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c >>> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c >>> index 069be3aaa5..55090e9c3e 100644 >>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c >>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c >>> @@ -125,18 +125,36 @@ GetPageTableEntry ( >>>UINTN Index2; >>>UINTN Index3; >>>UINTN Index4; >>> + UINTN Index5; >>>UINT64*L1PageTable; >>>UINT64*L2PageTable; >>>UINT64*L3PageTable; >>>UINT64*L4PageTable; >>> + UINT64*L5PageTable; >>> + IA32_CR4 Cr4; >>> + BOOLEAN Enable5LevelPaging; >>> >>> + Index5 = ((UINTN)RShiftU64 (Address, 48)) & >>> PAGING_PAE_INDEX_MASK; >>>Index4 = ((UINTN)RShiftU64 (Address, 39)) & >>> PAGING_PAE_INDEX_MASK; >>>Index3 = ((UINTN)Address >> 30) & >>> PAGING_PAE_INDEX_MASK; >>>Index2 = ((UINTN)Address >> 21) & >>> PAGING_PAE_INDEX_MASK; >>>Index1 = ((UINTN)Address >> 12
Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports
Mike, Thanks for raising this build failure. I just tried in my Ubuntu 18 in Win10. Even GCC7 complains about this. My bad! I just posted a fix. Thanks, Ray > -Original Message- > From: Kinney, Michael D > Sent: Thursday, July 11, 2019 4:06 AM > To: devel@edk2.groups.io; Ni, Ray ; Kinney, Michael D > > Cc: Dong, Eric ; Laszlo Ersek > Subject: RE: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level > paging when CPU supports > > Hi Ray, > > I noticed a Linux/GCC build issue with this patch when using GCC version: > > gcc version 8.2.1 20181215 (Red Hat 8.2.1-6) (GCC) > > edk2/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c: In function 'ReclaimPages': > edk2/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c:574:89: error: ?: using integer > constants in boolean context, the > expression will always evaluate to 'true' [-Werror=int-in-bool-context] >for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / > sizeof (*Pml4)) : 1; Pml5Index++) { > > I was able to get the build to pass if I added (). > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index c31160735a..a3b62f7787 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -571,7 +571,7 @@ ReclaimPages ( >// >// First, find the leaf entry has the smallest access record value >// > - for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / > sizeof (*Pml4)) : 1; Pml5Index++) { > + for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / > sizeof (*Pml4)) : 1); Pml5Index++) {^M > if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & > IA32_PG_PMNT) != 0) { >// >// If the PML5 entry is not present or is masked, skip it > > Best regards, > > Mike > > > -Original Message- > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] > > On Behalf Of Ni, Ray > > Sent: Tuesday, July 2, 2019 11:54 PM > > To: devel@edk2.groups.io > > Cc: Dong, Eric ; Laszlo Ersek > > > > Subject: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: > > Enable 5 level paging when CPU supports > > > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1946 > > > > The patch changes SMM environment to use 5 level paging > > when CPU > > supports it. > > > > Signed-off-by: Ray Ni > > Cc: Eric Dong > > Regression-tested-by: Laszlo Ersek > > --- > > .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 20 +- > > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c| 272 > > ++ > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 485 > > -- > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 12 + > > .../PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 72 ++- > > 5 files changed, 561 insertions(+), 300 deletions(-) > > > > diff --git > > a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > > b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > > index 069be3aaa5..55090e9c3e 100644 > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > > @@ -125,18 +125,36 @@ GetPageTableEntry ( > >UINTN Index2; > >UINTN Index3; > >UINTN Index4; > > + UINTN Index5; > >UINT64*L1PageTable; > >UINT64*L2PageTable; > >UINT64*L3PageTable; > >UINT64*L4PageTable; > > + UINT64*L5PageTable; > > + IA32_CR4 Cr4; > > + BOOLEAN Enable5LevelPaging; > > > > + Index5 = ((UINTN)RShiftU64 (Address, 48)) & > > PAGING_PAE_INDEX_MASK; > >Index4 = ((UINTN)RShiftU64 (Address, 39)) & > > PAGING_PAE_INDEX_MASK; > >Index3 = ((UINTN)Address >> 30) & > > PAGING_PAE_INDEX_MASK; > >Index2 = ((UINTN)Address >> 21) & > > PAGING_PAE_INDEX_MASK; > >Index1 = ((UINTN)Address >> 12) & > > PAGING_PAE_INDEX_MASK; > > > > + Cr4.UintN = AsmReadCr4 (); > > + Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1); > > + > >if (sizeof(UINTN) == sizeof(UINT64)) { > > -L4PageTable = (UINT64 *)GetPageTableBase (); > > +if (Enable5LevelPaging) { > > + L5PageTable = (UINT64 *)GetPageTableBase (); > > + if (L5PageTable[Index5] == 0) { > > +*PageAttribute = PageNone; > > +re
Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports
Hi Ray, I noticed a Linux/GCC build issue with this patch when using GCC version: gcc version 8.2.1 20181215 (Red Hat 8.2.1-6) (GCC) edk2/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c: In function 'ReclaimPages': edk2/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c:574:89: error: ?: using integer constants in boolean context, the expression will always evaluate to 'true' [-Werror=int-in-bool-context] for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1; Pml5Index++) { I was able to get the build to pass if I added (). diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c index c31160735a..a3b62f7787 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c @@ -571,7 +571,7 @@ ReclaimPages ( // // First, find the leaf entry has the smallest access record value // - for (Pml5Index = 0; Pml5Index < Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1; Pml5Index++) { + for (Pml5Index = 0; Pml5Index < (Enable5LevelPaging ? (EFI_PAGE_SIZE / sizeof (*Pml4)) : 1); Pml5Index++) {^M if ((Pml5[Pml5Index] & IA32_PG_P) == 0 || (Pml5[Pml5Index] & IA32_PG_PMNT) != 0) { // // If the PML5 entry is not present or is masked, skip it Best regards, Mike > -Original Message- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] > On Behalf Of Ni, Ray > Sent: Tuesday, July 2, 2019 11:54 PM > To: devel@edk2.groups.io > Cc: Dong, Eric ; Laszlo Ersek > > Subject: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: > Enable 5 level paging when CPU supports > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1946 > > The patch changes SMM environment to use 5 level paging > when CPU > supports it. > > Signed-off-by: Ray Ni > Cc: Eric Dong > Regression-tested-by: Laszlo Ersek > --- > .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 20 +- > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c| 272 > ++ > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 485 > -- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 12 + > .../PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 72 ++- > 5 files changed, 561 insertions(+), 300 deletions(-) > > diff --git > a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > index 069be3aaa5..55090e9c3e 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > @@ -125,18 +125,36 @@ GetPageTableEntry ( >UINTN Index2; >UINTN Index3; >UINTN Index4; > + UINTN Index5; >UINT64*L1PageTable; >UINT64*L2PageTable; >UINT64*L3PageTable; >UINT64*L4PageTable; > + UINT64*L5PageTable; > + IA32_CR4 Cr4; > + BOOLEAN Enable5LevelPaging; > > + Index5 = ((UINTN)RShiftU64 (Address, 48)) & > PAGING_PAE_INDEX_MASK; >Index4 = ((UINTN)RShiftU64 (Address, 39)) & > PAGING_PAE_INDEX_MASK; >Index3 = ((UINTN)Address >> 30) & > PAGING_PAE_INDEX_MASK; >Index2 = ((UINTN)Address >> 21) & > PAGING_PAE_INDEX_MASK; >Index1 = ((UINTN)Address >> 12) & > PAGING_PAE_INDEX_MASK; > > + Cr4.UintN = AsmReadCr4 (); > + Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1); > + >if (sizeof(UINTN) == sizeof(UINT64)) { > -L4PageTable = (UINT64 *)GetPageTableBase (); > +if (Enable5LevelPaging) { > + L5PageTable = (UINT64 *)GetPageTableBase (); > + if (L5PageTable[Index5] == 0) { > +*PageAttribute = PageNone; > +return NULL; > + } > + > + L4PageTable = (UINT64 > *)(UINTN)(L5PageTable[Index5] & ~mAddressEncMask & > PAGING_4K_ADDRESS_MASK_64); > +} else { > + L4PageTable = (UINT64 *)GetPageTableBase (); > +} > if (L4PageTable[Index4] == 0) { >*PageAttribute = PageNone; >return NULL; > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c > index e2b6a2d9b2..c5131526f0 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c > @@ -534,43 +534,78 @@ InitPaging ( >VOID >) > { > + UINT64Pml5Entry; > + UINT64Pml4Entry; > + UINT64*Pml5; >UINT64*Pml4; >UINT64*Pdpt; >UINT64*Pd; >UINT64*Pt; >UINTN Address; > + UINTN Pml5Index; >UINTN Pml4Index; >UINTN PdptIndex; >UINTN PdIndex; >UINTN PtIndex; >UINTN NumberOfPdptEntries; >UINTN
Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports
Reviewed-by: Eric Dong > -Original Message- > From: Ni, Ray > Sent: Wednesday, July 3, 2019 2:54 PM > To: devel@edk2.groups.io > Cc: Dong, Eric ; Laszlo Ersek > Subject: [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level paging when > CPU supports > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1946 > > The patch changes SMM environment to use 5 level paging when CPU > supports it. > > Signed-off-by: Ray Ni > Cc: Eric Dong > Regression-tested-by: Laszlo Ersek > --- > .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 20 +- > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c| 272 ++ > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 485 - > - > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 12 + > .../PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 72 ++- > 5 files changed, 561 insertions(+), 300 deletions(-) > > diff --git > a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > index 069be3aaa5..55090e9c3e 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c > @@ -125,18 +125,36 @@ GetPageTableEntry ( >UINTN Index2; >UINTN Index3; >UINTN Index4; > + UINTN Index5; >UINT64*L1PageTable; >UINT64*L2PageTable; >UINT64*L3PageTable; >UINT64*L4PageTable; > + UINT64*L5PageTable; > + IA32_CR4 Cr4; > + BOOLEAN Enable5LevelPaging; > > + Index5 = ((UINTN)RShiftU64 (Address, 48)) & PAGING_PAE_INDEX_MASK; >Index4 = ((UINTN)RShiftU64 (Address, 39)) & PAGING_PAE_INDEX_MASK; >Index3 = ((UINTN)Address >> 30) & PAGING_PAE_INDEX_MASK; >Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK; >Index1 = ((UINTN)Address >> 12) & PAGING_PAE_INDEX_MASK; > > + Cr4.UintN = AsmReadCr4 (); > + Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1); > + >if (sizeof(UINTN) == sizeof(UINT64)) { > -L4PageTable = (UINT64 *)GetPageTableBase (); > +if (Enable5LevelPaging) { > + L5PageTable = (UINT64 *)GetPageTableBase (); > + if (L5PageTable[Index5] == 0) { > +*PageAttribute = PageNone; > +return NULL; > + } > + > + L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] & > ~mAddressEncMask & PAGING_4K_ADDRESS_MASK_64); > +} else { > + L4PageTable = (UINT64 *)GetPageTableBase (); > +} > if (L4PageTable[Index4] == 0) { >*PageAttribute = PageNone; >return NULL; > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c > index e2b6a2d9b2..c5131526f0 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c > @@ -534,43 +534,78 @@ InitPaging ( >VOID >) > { > + UINT64Pml5Entry; > + UINT64Pml4Entry; > + UINT64*Pml5; >UINT64*Pml4; >UINT64*Pdpt; >UINT64*Pd; >UINT64*Pt; >UINTN Address; > + UINTN Pml5Index; >UINTN Pml4Index; >UINTN PdptIndex; >UINTN PdIndex; >UINTN PtIndex; >UINTN NumberOfPdptEntries; >UINTN NumberOfPml4Entries; > + UINTN NumberOfPml5Entries; >UINTN SizeOfMemorySpace; >BOOLEAN Nx; > + IA32_CR4 Cr4; > + BOOLEAN Enable5LevelPaging; > + > + Cr4.UintN = AsmReadCr4 (); > + Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1); > >if (sizeof (UINTN) == sizeof (UINT64)) { > -Pml4 = (UINT64*)(UINTN)mSmmProfileCr3; > +if (!Enable5LevelPaging) { > + Pml5Entry = (UINTN) mSmmProfileCr3 | IA32_PG_P; > + Pml5 = &Pml5Entry; > +} else { > + Pml5 = (UINT64*) (UINTN) mSmmProfileCr3; > +} > SizeOfMemorySpace = HighBitSet64 (gPhyMask) + 1; > // > // Calculate the table entries of PML4E and PDPTE. > // > -if (SizeOfMemorySpace <= 39 ) { > - NumberOfPml4Entries = 1; > - NumberOfPdptEntries = (UINT32)LShiftU64 (1, (SizeOfMemorySpace - > 30)); > -} else { > - NumberOfPml4Entries = (UINT32)LShiftU64 (1, (SizeOfMemorySpace - > 39)); > - NumberOfPdptEntries = 512; > +NumberOfPml5Entries = 1; > +if (SizeOfMemorySpace > 48) { > + NumberOfPml5Entries = (UINTN) LShiftU64 (1, SizeOfMemorySpace - 48); > + SizeOfMemorySpace = 48; > } > - } else { > + > NumberOfPml4Entries = 1; > +if (SizeOfMemorySpa