Re: [edk2] [patch 1/2] UefiCpuPkg/PiSmmCpu: Always set RW+P bit for page table by default.

2015-11-27 Thread Laszlo Ersek 
On 11/27/15 15:07, Yao, Jiewen wrote:
> So quick!
> Thank you very much to catch this!

You'll get used to Paolo... the only reason he opens the SDM is not
because he needs to look up the details. He remembers those. He looks at
the SDM in order to find the containing chapter & section numbers for
the details he already knows, so that others can look up the reference too.

... That's my impression anyway! ;)

Laszlo

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Paolo 
> Bonzini
> Sent: Friday, November 27, 2015 9:53 PM
> To: Yao, Jiewen; edk2-de...@ml01.01.org
> Cc: Kinney, Michael D; Fan, Jeff
> Subject: Re: [edk2] [patch 1/2] UefiCpuPkg/PiSmmCpu: Always set RW+P bit for 
> page table by default.
> 
> 
> 
> On 25/11/2015 13:34, jiewen yao wrote:
>> @@ -785,7 +785,7 @@ Gen4GPageTable (
>>// Set Page Directory Pointers
>>//
>>for (Index = 0; Index < 4; Index++) {
>> -Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + IA32_PG_P;
>> +Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + 
>> + PAGE_ATTRIBUTE_BITS;
>>}
>>Pte += EFI_PAGE_SIZE / sizeof (*Pte);
>>  
> 
> For PAE you must not set bit 1 here.  See the fix I've posted, it can be 
> included directly in this patch when you repost (so you still have two 
> patches, "Always set RW+P bit" and "Always set WP").
> 
> Thanks,
> 
> Paolo
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch 1/2] UefiCpuPkg/PiSmmCpu: Always set RW+P bit for page table by default.

2015-11-27 Thread Paolo Bonzini 


On 27/11/2015 15:21, Laszlo Ersek wrote:
> On 11/27/15 15:07, Yao, Jiewen wrote:
>> > So quick!
>> > Thank you very much to catch this!
> You'll get used to Paolo... the only reason he opens the SDM is not
> because he needs to look up the details. He remembers those. He looks at
> the SDM in order to find the containing chapter & section numbers for
> the details he already knows, so that others can look up the reference too.
> 
> ... That's my impression anyway! ;)

Not really, but the next time you get a failed vmentry, check out the
VMCS dump in dmesg.  All the information you need is there, you "just"
have to check it against the list in the SDM.

In this case, CR0.WP was a red herring, as the same mov to cr0 was
setting CR0.PG as well.

Paolo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch 1/2] UefiCpuPkg/PiSmmCpu: Always set RW+P bit for page table by default.

2015-11-27 Thread Paolo Bonzini 


On 25/11/2015 13:34, jiewen yao wrote:
> @@ -785,7 +785,7 @@ Gen4GPageTable (
>// Set Page Directory Pointers
>//
>for (Index = 0; Index < 4; Index++) {
> -Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + IA32_PG_P;
> +Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + 
> PAGE_ATTRIBUTE_BITS;
>}
>Pte += EFI_PAGE_SIZE / sizeof (*Pte);
>  

For PAE you must not set bit 1 here.  See the fix I've posted, it can be
included directly in this patch when you repost (so you still have two
patches, "Always set RW+P bit" and "Always set WP").

Thanks,

Paolo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [patch 1/2] UefiCpuPkg/PiSmmCpu: Always set RW+P bit for page table by default.

2015-11-27 Thread jiewen yao 
So that we can use write-protection for code later.

This is REPOST.
It includes the bug fix from "Paolo Bonzini" .
Title: fix generation of 32-bit PAE page tables
Bits 1 and 2 are reserved in 32-bit PAE Page Directory Pointer Table
Entries (PDPTEs); see Table 4-8 in the SDM.  With VMX extended page
tables, the processor notices and fails the VM entry as soon as
CR0.PG is set to 1.

And thanks "Laszlo Ersek"  to validate the fix.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: "Yao, Jiewen" 
Signed-off-by: "Paolo Bonzini" 
Tested-by: "Laszlo Ersek" 
Reviewed-by: "Kinney, Michael D" 
Cc: "Fan, Jeff" 
Cc: "Kinney, Michael D" 
Cc: "Laszlo Ersek" 
Cc: "Paolo Bonzini" 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c|  2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c |  2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c   | 14 --
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h  | 13 -
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c  | 12 ++--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c |  8 
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c  | 14 +++---
 7 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index edebaab..5d29904 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -60,7 +60,7 @@ SmmInitPageTable (
   if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
 InitializeIDTSmmStackGuard ();
   }
-  return Gen4GPageTable (0);
+  return Gen4GPageTable (0, TRUE);
 }
 
 /**
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c
index 85756d0..767cb69 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c
@@ -24,7 +24,7 @@ InitSmmS3Cr3 (
   VOID
   )
 {
-  mSmmS3ResumeState->SmmS3Cr3 = Gen4GPageTable (0);
+  mSmmS3ResumeState->SmmS3Cr3 = Gen4GPageTable (0, TRUE);
 
   return ;
 }
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 06ffc6d..620b013 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -732,12 +732,14 @@ APHandler (
   Create 4G PageTable in SMRAM.
 
   @param  ExtraPages   Additional page numbers besides for 4G 
memory
+  @param  Is32BitPageTable Whether the page table is 32-bit PAE
   @return PageTable Address
 
 **/
 UINT32
 Gen4GPageTable (
-  IN  UINTN ExtraPages
+  IN  UINTN ExtraPages,
+  IN  BOOLEAN   Is32BitPageTable
   )
 {
   VOID*PageTable;
@@ -785,7 +787,7 @@ Gen4GPageTable (
   // Set Page Directory Pointers
   //
   for (Index = 0; Index < 4; Index++) {
-Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + IA32_PG_P;
+Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + 
(Is32BitPageTable ? IA32_PAE_PDPTE_ATTRIBUTE_BITS : PAGE_ATTRIBUTE_BITS);
   }
   Pte += EFI_PAGE_SIZE / sizeof (*Pte);
 
@@ -793,7 +795,7 @@ Gen4GPageTable (
   // Fill in Page Directory Entries
   //
   for (Index = 0; Index < EFI_PAGE_SIZE * 4 / sizeof (*Pte); Index++) {
-Pte[Index] = (Index << 21) + IA32_PG_PS + IA32_PG_RW + IA32_PG_P;
+Pte[Index] = (Index << 21) | IA32_PG_PS | PAGE_ATTRIBUTE_BITS;
   }
 
   if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
@@ -802,7 +804,7 @@ Gen4GPageTable (
 Pdpte = (UINT64*)PageTable;
 for (PageIndex = Low2MBoundary; PageIndex <= High2MBoundary; PageIndex += 
SIZE_2MB) {
   Pte = (UINT64*)(UINTN)(Pdpte[BitFieldRead32 ((UINT32)PageIndex, 30, 31)] 
& ~(EFI_PAGE_SIZE - 1));
-  Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages + 
IA32_PG_RW + IA32_PG_P;
+  Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages | 
PAGE_ATTRIBUTE_BITS;
   //
   // Fill in Page Table Entries
   //
@@ -819,7 +821,7 @@ Gen4GPageTable (
 GuardPage = 0;
   }
 } else {
-  Pte[Index] = PageAddress + IA32_PG_RW + IA32_PG_P;
+  Pte[Index] = PageAddress | PAGE_ATTRIBUTE_BITS;
 }
 PageAddress+= EFI_PAGE_SIZE;
   }
@@ -886,7 +888,7 @@ SetCacheability (
   NewPageTable[Index] |= (UINT64)(Index << EFI_PAGE_SHIFT);
 }
 
-PageTable[PTIndex] = ((UINTN)NewPageTableAddress & gPhyMask) | IA32_PG_P;
+PageTable[PTIndex] = ((UINTN)NewPageTableAddress & gPhyMask) | 
PAGE_ATTRIBUTE_BITS;
   }
 
   ASSERT (PageTable[PTIndex] & IA32_PG_P);
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index f2a9165..9920cd1 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h

Re: [edk2] [patch 1/2] UefiCpuPkg/PiSmmCpu: Always set RW+P bit for page table by default.

2015-11-25 Thread Kinney, Michael D 
Reviewed-by: Michael Kinney 

> -Original Message-
> From: Yao, Jiewen
> Sent: Wednesday, November 25, 2015 4:35 AM
> To: edk2-de...@ml01.01.org
> Cc: Yao, Jiewen ; Fan, Jeff ; 
> Kinney, Michael D 
> Subject: [patch 1/2] UefiCpuPkg/PiSmmCpu: Always set RW+P bit for page table 
> by default.
> 
> So that we can use write-protection for code later.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: "Yao, Jiewen" 
> Cc: "Fan, Jeff" 
> Cc: "Kinney, Michael D" 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  | 10 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  4 
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 12 ++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c|  6 +++---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 12 ++--
>  5 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> index 26c9a0f..084d217 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
> @@ -785,7 +785,7 @@ Gen4GPageTable (
>// Set Page Directory Pointers
>//
>for (Index = 0; Index < 4; Index++) {
> -Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + IA32_PG_P;
> +Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + 
> PAGE_ATTRIBUTE_BITS;
>}
>Pte += EFI_PAGE_SIZE / sizeof (*Pte);
> 
> @@ -793,7 +793,7 @@ Gen4GPageTable (
>// Fill in Page Directory Entries
>//
>for (Index = 0; Index < EFI_PAGE_SIZE * 4 / sizeof (*Pte); Index++) {
> -Pte[Index] = (Index << 21) + IA32_PG_PS + IA32_PG_RW + IA32_PG_P;
> +Pte[Index] = (Index << 21) | IA32_PG_PS | PAGE_ATTRIBUTE_BITS;
>}
> 
>if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
> @@ -802,7 +802,7 @@ Gen4GPageTable (
>  Pdpte = (UINT64*)PageTable;
>  for (PageIndex = Low2MBoundary; PageIndex <= High2MBoundary; PageIndex 
> += SIZE_2MB) {
>Pte = (UINT64*)(UINTN)(Pdpte[BitFieldRead32 ((UINT32)PageIndex, 30, 
> 31)] & ~(EFI_PAGE_SIZE - 1));
> -  Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages + 
> IA32_PG_RW + IA32_PG_P;
> +  Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages | 
> PAGE_ATTRIBUTE_BITS;
>//
>// Fill in Page Table Entries
>//
> @@ -819,7 +819,7 @@ Gen4GPageTable (
>  GuardPage = 0;
>}
>  } else {
> -  Pte[Index] = PageAddress + IA32_PG_RW + IA32_PG_P;
> +  Pte[Index] = PageAddress | PAGE_ATTRIBUTE_BITS;
>  }
>  PageAddress+= EFI_PAGE_SIZE;
>}
> @@ -886,7 +886,7 @@ SetCacheability (
>NewPageTable[Index] |= (UINT64)(Index << EFI_PAGE_SHIFT);
>  }
> 
> -PageTable[PTIndex] = ((UINTN)NewPageTableAddress & gPhyMask) | IA32_PG_P;
> +PageTable[PTIndex] = ((UINTN)NewPageTableAddress & gPhyMask) | 
> PAGE_ATTRIBUTE_BITS;
>}
> 
>ASSERT (PageTable[PTIndex] & IA32_PG_P);
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 9caccb5..cce2a09 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -71,15 +71,19 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  ///
>  #define IA32_PG_P   BIT0
>  #define IA32_PG_RW  BIT1
> +#define IA32_PG_U   BIT2
>  #define IA32_PG_WT  BIT3
>  #define IA32_PG_CD  BIT4
>  #define IA32_PG_A   BIT5
> +#define IA32_PG_D   BIT6
>  #define IA32_PG_PS  BIT7
>  #define IA32_PG_PAT_2M  BIT12
>  #define IA32_PG_PAT_4K  IA32_PG_PS
>  #define IA32_PG_PMNTBIT62
>  #define IA32_PG_NX  BIT63
> 
> +#define PAGE_ATTRIBUTE_BITS (IA32_PG_RW | IA32_PG_P)
> +
>  //
>  // Size of Task-State Segment defined in IA32 Manual
>  //
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index ff4e28e..ec4ec9b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -557,9 +557,9 @@ InitPaging (
> 
>// Split it
>for (Level4 = 0; Level4 < SIZE_4KB / sizeof(*Pt); Level4++) {
> -Pt[Level4] = Address + ((Level4 << 12) | IA32_PG_RW | IA32_PG_P);
> +Pt[Level4] = Address + ((Level4 << 12) | PAGE_ATTRIBUTE_BITS);
>} // end for PT
> -  *Pte = (UINTN)Pt | IA32_PG_RW | IA32_PG_P;
> +  *Pte = (UINTN)Pt | PAGE_ATTRIBUTE_BITS;
>  } // end if IsAddressSplit
>} // end for PTE
>  } // end for PDE
> @@ -608,7 +608,7 @@ InitPaging (
>  //
>

[edk2] [patch 1/2] UefiCpuPkg/PiSmmCpu: Always set RW+P bit for page table by default.

2015-11-25 Thread jiewen yao 
So that we can use write-protection for code later.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: "Yao, Jiewen" 
Cc: "Fan, Jeff" 
Cc: "Kinney, Michael D" 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  | 10 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  4 
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 12 ++--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c|  6 +++---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 12 ++--
 5 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
index 26c9a0f..084d217 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c
@@ -785,7 +785,7 @@ Gen4GPageTable (
   // Set Page Directory Pointers
   //
   for (Index = 0; Index < 4; Index++) {
-Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + IA32_PG_P;
+Pte[Index] = (UINTN)PageTable + EFI_PAGE_SIZE * (Index + 1) + 
PAGE_ATTRIBUTE_BITS;
   }
   Pte += EFI_PAGE_SIZE / sizeof (*Pte);
 
@@ -793,7 +793,7 @@ Gen4GPageTable (
   // Fill in Page Directory Entries
   //
   for (Index = 0; Index < EFI_PAGE_SIZE * 4 / sizeof (*Pte); Index++) {
-Pte[Index] = (Index << 21) + IA32_PG_PS + IA32_PG_RW + IA32_PG_P;
+Pte[Index] = (Index << 21) | IA32_PG_PS | PAGE_ATTRIBUTE_BITS;
   }
 
   if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
@@ -802,7 +802,7 @@ Gen4GPageTable (
 Pdpte = (UINT64*)PageTable;
 for (PageIndex = Low2MBoundary; PageIndex <= High2MBoundary; PageIndex += 
SIZE_2MB) {
   Pte = (UINT64*)(UINTN)(Pdpte[BitFieldRead32 ((UINT32)PageIndex, 30, 31)] 
& ~(EFI_PAGE_SIZE - 1));
-  Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages + 
IA32_PG_RW + IA32_PG_P;
+  Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages | 
PAGE_ATTRIBUTE_BITS;
   //
   // Fill in Page Table Entries
   //
@@ -819,7 +819,7 @@ Gen4GPageTable (
 GuardPage = 0;
   }
 } else {
-  Pte[Index] = PageAddress + IA32_PG_RW + IA32_PG_P;
+  Pte[Index] = PageAddress | PAGE_ATTRIBUTE_BITS;
 }
 PageAddress+= EFI_PAGE_SIZE;
   }
@@ -886,7 +886,7 @@ SetCacheability (
   NewPageTable[Index] |= (UINT64)(Index << EFI_PAGE_SHIFT);
 }
 
-PageTable[PTIndex] = ((UINTN)NewPageTableAddress & gPhyMask) | IA32_PG_P;
+PageTable[PTIndex] = ((UINTN)NewPageTableAddress & gPhyMask) | 
PAGE_ATTRIBUTE_BITS;
   }
 
   ASSERT (PageTable[PTIndex] & IA32_PG_P);
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 9caccb5..cce2a09 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -71,15 +71,19 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 ///
 #define IA32_PG_P   BIT0
 #define IA32_PG_RW  BIT1
+#define IA32_PG_U   BIT2
 #define IA32_PG_WT  BIT3
 #define IA32_PG_CD  BIT4
 #define IA32_PG_A   BIT5
+#define IA32_PG_D   BIT6
 #define IA32_PG_PS  BIT7
 #define IA32_PG_PAT_2M  BIT12
 #define IA32_PG_PAT_4K  IA32_PG_PS
 #define IA32_PG_PMNTBIT62
 #define IA32_PG_NX  BIT63
 
+#define PAGE_ATTRIBUTE_BITS (IA32_PG_RW | IA32_PG_P)
+
 //
 // Size of Task-State Segment defined in IA32 Manual
 //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index ff4e28e..ec4ec9b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -557,9 +557,9 @@ InitPaging (
 
   // Split it
   for (Level4 = 0; Level4 < SIZE_4KB / sizeof(*Pt); Level4++) {
-Pt[Level4] = Address + ((Level4 << 12) | IA32_PG_RW | IA32_PG_P);
+Pt[Level4] = Address + ((Level4 << 12) | PAGE_ATTRIBUTE_BITS);
   } // end for PT
-  *Pte = (UINTN)Pt | IA32_PG_RW | IA32_PG_P;
+  *Pte = (UINTN)Pt | PAGE_ATTRIBUTE_BITS;
 } // end if IsAddressSplit
   } // end for PTE
 } // end for PDE
@@ -608,7 +608,7 @@ InitPaging (
 //
 // Patch to remove Present flag and RW flag
 //
-*Pte = *Pte & (INTN)(INT32)(~(IA32_PG_RW | IA32_PG_P));
+*Pte = *Pte & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
   }
   if (Nx && mXdSupported) {
 *Pte = *Pte | IA32_PG_NX;
@@ -621,7 +621,7 @@ InitPaging (
   }
   for (Level4 = 0; Level4 < SIZE_4KB / sizeof(*Pt); Level4++, Pt++) {
 if (!IsAddressValid (Address, )) {
-  *Pt = *Pt & (INTN)(INT32)(~(IA32_PG_RW | IA32_PG_P));
+  *Pt = *Pt & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
 }
 if