public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
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
> 
> 
> 


  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