From: "Dong, Eric" <eric.dong@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"Ni, Ray" <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Subject: Re: [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpu: Change variable names and comments to follow SDM
Date: Tue, 9 Jul 2019 01:08:35 +0000 [thread overview]
Message-ID: <ED077930C258884BBCB450DB737E662259E92971@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20190703065416.116816-2-ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni,
> Ray
> Sent: Wednesday, July 3, 2019 2:54 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [edk2-devel] [PATCH v2 1/3] UefiCpuPkg/PiSmmCpu: Change
> variable names and comments to follow SDM
>
> Per SDM, for IA-32e 4-KByte paging, there are four layers in the page table
> structure:
> 1. PML4
> 2. Page-Directory-Pointer Table (PDPT)
> 3. Page-Directory (PD)
> 4. Page Table (PT)
>
> The patch changes the local variable names and comments to use "PML4",
> "PDPT", "PD", "PT" to better align to terms used in SDM.
>
> There is no functionality impact for this change.
>
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 114 ++++++++++++---------
> ----
> 1 file changed, 57 insertions(+), 57 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index 3b2f967355..e2b6a2d9b2 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -535,15 +535,15 @@ InitPaging (
> )
> {
> UINT64 *Pml4;
> - UINT64 *Pde;
> - UINT64 *Pte;
> + UINT64 *Pdpt;
> + UINT64 *Pd;
> UINT64 *Pt;
> UINTN Address;
> - UINTN Level1;
> - UINTN Level2;
> - UINTN Level3;
> - UINTN Level4;
> - UINTN NumberOfPdpEntries;
> + UINTN Pml4Index;
> + UINTN PdptIndex;
> + UINTN PdIndex;
> + UINTN PtIndex;
> + UINTN NumberOfPdptEntries;
> UINTN NumberOfPml4Entries;
> UINTN SizeOfMemorySpace;
> BOOLEAN Nx;
> @@ -556,143 +556,143 @@ InitPaging (
> //
> if (SizeOfMemorySpace <= 39 ) {
> NumberOfPml4Entries = 1;
> - NumberOfPdpEntries = (UINT32)LShiftU64 (1, (SizeOfMemorySpace -
> 30));
> + NumberOfPdptEntries = (UINT32)LShiftU64 (1, (SizeOfMemorySpace -
> + 30));
> } else {
> NumberOfPml4Entries = (UINT32)LShiftU64 (1, (SizeOfMemorySpace -
> 39));
> - NumberOfPdpEntries = 512;
> + NumberOfPdptEntries = 512;
> }
> } else {
> NumberOfPml4Entries = 1;
> - NumberOfPdpEntries = 4;
> + NumberOfPdptEntries = 4;
> }
>
> //
> // Go through page table and change 2MB-page into 4KB-page.
> //
> - for (Level1 = 0; Level1 < NumberOfPml4Entries; Level1++) {
> + for (Pml4Index = 0; Pml4Index < NumberOfPml4Entries; Pml4Index++) {
> if (sizeof (UINTN) == sizeof (UINT64)) {
> - if ((Pml4[Level1] & IA32_PG_P) == 0) {
> + if ((Pml4[Pml4Index] & IA32_PG_P) == 0) {
> //
> - // If Pml4 entry does not exist, skip it
> + // If PML4 entry does not exist, skip it
> //
> continue;
> }
> - Pde = (UINT64 *)(UINTN)(Pml4[Level1] & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> + Pdpt = (UINT64 *)(UINTN)(Pml4[Pml4Index] & ~mAddressEncMask &
> + PHYSICAL_ADDRESS_MASK);
> } else {
> - Pde = (UINT64*)(UINTN)mSmmProfileCr3;
> + Pdpt = (UINT64*)(UINTN)mSmmProfileCr3;
> }
> - for (Level2 = 0; Level2 < NumberOfPdpEntries; Level2++, Pde++) {
> - if ((*Pde & IA32_PG_P) == 0) {
> + for (PdptIndex = 0; PdptIndex < NumberOfPdptEntries; PdptIndex++,
> Pdpt++) {
> + if ((*Pdpt & IA32_PG_P) == 0) {
> //
> - // If PDE entry does not exist, skip it
> + // If PDPT entry does not exist, skip it
> //
> continue;
> }
> - if ((*Pde & IA32_PG_PS) != 0) {
> + if ((*Pdpt & IA32_PG_PS) != 0) {
> //
> // This is 1G entry, skip it
> //
> continue;
> }
> - Pte = (UINT64 *)(UINTN)(*Pde & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> - if (Pte == 0) {
> + Pd = (UINT64 *)(UINTN)(*Pdpt & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> + if (Pd == 0) {
> continue;
> }
> - for (Level3 = 0; Level3 < SIZE_4KB / sizeof (*Pte); Level3++, Pte++) {
> - if ((*Pte & IA32_PG_P) == 0) {
> + for (PdIndex = 0; PdIndex < SIZE_4KB / sizeof (*Pd); PdIndex++, Pd++) {
> + if ((*Pd & IA32_PG_P) == 0) {
> //
> - // If PTE entry does not exist, skip it
> + // If PD entry does not exist, skip it
> //
> continue;
> }
> - Address = (((Level2 << 9) + Level3) << 21);
> + Address = (((PdptIndex << 9) + PdIndex) << 21);
>
> //
> // If it is 2M page, check IsAddressSplit()
> //
> - if (((*Pte & IA32_PG_PS) != 0) && IsAddressSplit (Address)) {
> + if (((*Pd & IA32_PG_PS) != 0) && IsAddressSplit (Address)) {
> //
> // Based on current page table, create 4KB page table for split area.
> //
> - ASSERT (Address == (*Pte & PHYSICAL_ADDRESS_MASK));
> + ASSERT (Address == (*Pd & PHYSICAL_ADDRESS_MASK));
>
> Pt = AllocatePageTableMemory (1);
> ASSERT (Pt != NULL);
>
> // Split it
> - for (Level4 = 0; Level4 < SIZE_4KB / sizeof(*Pt); Level4++) {
> - Pt[Level4] = Address + ((Level4 << 12) | mAddressEncMask |
> PAGE_ATTRIBUTE_BITS);
> + for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof(*Pt); PtIndex++) {
> + Pt[PtIndex] = Address + ((PtIndex << 12) | mAddressEncMask
> + | PAGE_ATTRIBUTE_BITS);
> } // end for PT
> - *Pte = (UINT64)(UINTN)Pt | mAddressEncMask |
> PAGE_ATTRIBUTE_BITS;
> + *Pd = (UINT64)(UINTN)Pt | mAddressEncMask |
> + PAGE_ATTRIBUTE_BITS;
> } // end if IsAddressSplit
> - } // end for PTE
> - } // end for PDE
> - }
> + } // end for PD
> + } // end for PDPT
> + } // end for PML4
>
> //
> // Go through page table and set several page table entries to absent or
> execute-disable.
> //
> DEBUG ((EFI_D_INFO, "Patch page table start ...\n"));
> - for (Level1 = 0; Level1 < NumberOfPml4Entries; Level1++) {
> + for (Pml4Index = 0; Pml4Index < NumberOfPml4Entries; Pml4Index++) {
> if (sizeof (UINTN) == sizeof (UINT64)) {
> - if ((Pml4[Level1] & IA32_PG_P) == 0) {
> + if ((Pml4[Pml4Index] & IA32_PG_P) == 0) {
> //
> - // If Pml4 entry does not exist, skip it
> + // If PML4 entry does not exist, skip it
> //
> continue;
> }
> - Pde = (UINT64 *)(UINTN)(Pml4[Level1] & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> + Pdpt = (UINT64 *)(UINTN)(Pml4[Pml4Index] & ~mAddressEncMask &
> + PHYSICAL_ADDRESS_MASK);
> } else {
> - Pde = (UINT64*)(UINTN)mSmmProfileCr3;
> + Pdpt = (UINT64*)(UINTN)mSmmProfileCr3;
> }
> - for (Level2 = 0; Level2 < NumberOfPdpEntries; Level2++, Pde++) {
> - if ((*Pde & IA32_PG_P) == 0) {
> + for (PdptIndex = 0; PdptIndex < NumberOfPdptEntries; PdptIndex++,
> Pdpt++) {
> + if ((*Pdpt & IA32_PG_P) == 0) {
> //
> - // If PDE entry does not exist, skip it
> + // If PDPT entry does not exist, skip it
> //
> continue;
> }
> - if ((*Pde & IA32_PG_PS) != 0) {
> + if ((*Pdpt & IA32_PG_PS) != 0) {
> //
> // This is 1G entry, set NX bit and skip it
> //
> if (mXdSupported) {
> - *Pde = *Pde | IA32_PG_NX;
> + *Pdpt = *Pdpt | IA32_PG_NX;
> }
> continue;
> }
> - Pte = (UINT64 *)(UINTN)(*Pde & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> - if (Pte == 0) {
> + Pd = (UINT64 *)(UINTN)(*Pdpt & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> + if (Pd == 0) {
> continue;
> }
> - for (Level3 = 0; Level3 < SIZE_4KB / sizeof (*Pte); Level3++, Pte++) {
> - if ((*Pte & IA32_PG_P) == 0) {
> + for (PdIndex = 0; PdIndex < SIZE_4KB / sizeof (*Pd); PdIndex++, Pd++) {
> + if ((*Pd & IA32_PG_P) == 0) {
> //
> - // If PTE entry does not exist, skip it
> + // If PD entry does not exist, skip it
> //
> continue;
> }
> - Address = (((Level2 << 9) + Level3) << 21);
> + Address = (((PdptIndex << 9) + PdIndex) << 21);
>
> - if ((*Pte & IA32_PG_PS) != 0) {
> + if ((*Pd & IA32_PG_PS) != 0) {
> // 2MB page
>
> if (!IsAddressValid (Address, &Nx)) {
> //
> // Patch to remove Present flag and RW flag
> //
> - *Pte = *Pte & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
> + *Pd = *Pd & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
> }
> if (Nx && mXdSupported) {
> - *Pte = *Pte | IA32_PG_NX;
> + *Pd = *Pd | IA32_PG_NX;
> }
> } else {
> // 4KB page
> - Pt = (UINT64 *)(UINTN)(*Pte & ~mAddressEncMask &
> PHYSICAL_ADDRESS_MASK);
> + Pt = (UINT64 *)(UINTN)(*Pd & ~mAddressEncMask &
> + PHYSICAL_ADDRESS_MASK);
> if (Pt == 0) {
> continue;
> }
> - for (Level4 = 0; Level4 < SIZE_4KB / sizeof(*Pt); Level4++, Pt++) {
> + for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof(*Pt);
> + PtIndex++, Pt++) {
> if (!IsAddressValid (Address, &Nx)) {
> *Pt = *Pt & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
> }
> @@ -702,9 +702,9 @@ InitPaging (
> Address += SIZE_4KB;
> } // end for PT
> } // end if PS
> - } // end for PTE
> - } // end for PDE
> - }
> + } // end for PD
> + } // end for PDPT
> + } // end for PML4
>
> //
> // Flush TLB
> --
> 2.21.0.windows.1
>
>
>
next prev parent reply other threads:[~2019-07-09 1:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-03 6:54 [PATCH v2 0/3] Enable 5 level paging in SMM mode Ni, Ray
2019-07-03 6:54 ` [PATCH v2 1/3] UefiCpuPkg/PiSmmCpu: Change variable names and comments to follow SDM Ni, Ray
2019-07-09 1:08 ` Dong, Eric [this message]
2019-07-03 6:54 ` [PATCH v2 2/3] MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level paging Ni, Ray
2019-07-09 1:04 ` Dong, Eric
2019-07-10 17:17 ` [edk2-devel] " Laszlo Ersek
2019-07-10 19:38 ` Michael D Kinney
2019-07-11 3:25 ` Ni, Ray
2019-07-11 4:05 ` Ni, Ray
2019-07-11 12:12 ` Laszlo Ersek
2019-07-11 12:29 ` Laszlo Ersek
2019-07-11 12:17 ` Laszlo Ersek
2019-07-03 6:54 ` [PATCH v2 3/3] UefiCpuPkg/PiSmmCpu: Enable 5 level paging when CPU supports Ni, Ray
2019-07-09 13:33 ` Dong, Eric
2019-07-10 20:06 ` [edk2-devel] " Michael D Kinney
2019-07-11 1:19 ` Ni, Ray
2019-07-11 12:17 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ED077930C258884BBCB450DB737E662259E92971@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox