Reviewed-by: Michael Kinney <michael.d.kin...@intel.com>
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Wednesday, November 25, 2015 4:35 AM
> To: edk2-de...@ml01.01.org
> Cc: Yao, Jiewen <jiewen....@intel.com>; Fan, Jeff <jeff....@intel.com>;
> Kinney, Michael D <michael.d.kin...@intel.com>
> 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" <jiewen....@intel.com>
> Cc: "Fan, Jeff" <jeff....@intel.com>
> Cc: "Kinney, Michael D" <michael.d.kin...@intel.com>
> ---
> 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_PMNT BIT62
> #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, &Nx)) {
> - *Pt = *Pt & (INTN)(INT32)(~(IA32_PG_RW | IA32_PG_P));
> + *Pt = *Pt & (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
> }
> if (Nx && mXdSupported) {
> *Pt = *Pt | IA32_PG_NX;
> @@ -1244,7 +1244,7 @@ RestorePageTableBelow4G (
> //
> PageTable[PTIndex] = (PFAddress & ~((1ull << 21) - 1));
> PageTable[PTIndex] |= (UINT64)IA32_PG_PS;
> - PageTable[PTIndex] |= (UINT64)(IA32_PG_RW | IA32_PG_P);
> + PageTable[PTIndex] |= (UINT64)PAGE_ATTRIBUTE_BITS;
> if ((ErrorCode & IA32_PF_EC_ID) != 0) {
> PageTable[PTIndex] &= ~IA32_PG_NX;
> }
> @@ -1277,7 +1277,7 @@ RestorePageTableBelow4G (
> // Set new entry
> //
> PageTable[PTIndex] = (PFAddress & ~((1ull << 12) - 1));
> - PageTable[PTIndex] |= (UINT64)(IA32_PG_RW | IA32_PG_P);
> + PageTable[PTIndex] |= (UINT64)PAGE_ATTRIBUTE_BITS;
> if ((ErrorCode & IA32_PF_EC_ID) != 0) {
> PageTable[PTIndex] &= ~IA32_PG_NX;
> }
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index a7d790f..d242e06 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -127,7 +127,7 @@ SmmInitPageTable (
> // Fill Page-Table-Level4 (PML4) entry
> //
> PTEntry = (UINT64*)(UINTN)(Pages - EFI_PAGES_TO_SIZE (PAGE_TABLE_PAGES +
> 1));
> - *PTEntry = Pages + IA32_PG_P;
> + *PTEntry = Pages + PAGE_ATTRIBUTE_BITS;
> ZeroMem (PTEntry + 1, EFI_PAGE_SIZE - sizeof (*PTEntry));
> //
> // Set sub-entries number
> @@ -591,7 +591,7 @@ SmiDefaultPFHandler (
> //
> // If the entry is not present, allocate one page from page pool for
> it
> //
> - PageTable[PTIndex] = AllocPage () | IA32_PG_RW | IA32_PG_P;
> + PageTable[PTIndex] = AllocPage () | PAGE_ATTRIBUTE_BITS;
> } else {
> //
> // Save the upper entry address
> @@ -621,7 +621,7 @@ SmiDefaultPFHandler (
> // Fill the new entry
> //
> PageTable[PTIndex] = (PFAddress & gPhyMask & ~((1ull << EndBit) - 1)) |
> - PageAttribute | IA32_PG_A | IA32_PG_RW | IA32_PG_P;
> + PageAttribute | IA32_PG_A | PAGE_ATTRIBUTE_BITS;
> if (UpperEntry != NULL) {
> SetSubEntriesNum (UpperEntry, GetSubEntriesNum (UpperEntry) + 1);
> }
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c
> index c4ec12d..b3cd629 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c
> @@ -51,7 +51,7 @@ InitSmmS3Cr3 (
> // Fill Page-Table-Level4 (PML4) entry
> //
> PTEntry = (UINT64*)(UINTN)(Pages - EFI_PAGES_TO_SIZE (1));
> - *PTEntry = Pages + IA32_PG_P;
> + *PTEntry = Pages | PAGE_ATTRIBUTE_BITS;
> ZeroMem (PTEntry + 1, EFI_PAGE_SIZE - sizeof (*PTEntry));
>
> //
> @@ -117,7 +117,7 @@ AcquirePage (
> //
> // Link & Record the current uplink
> //
> - *Uplink = Address | IA32_PG_P | IA32_PG_RW;
> + *Uplink = Address | PAGE_ATTRIBUTE_BITS;
> mPFPageUplink[mPFPageIndex] = Uplink;
>
> mPFPageIndex = (mPFPageIndex + 1) % MAX_PF_PAGE_COUNT;
> @@ -242,9 +242,9 @@ RestorePageTableAbove4G (
> // PTE
> PageTable = (UINT64*)(UINTN)(PageTable[PTIndex] &
> PHYSICAL_ADDRESS_MASK);
> for (Index = 0; Index < 512; Index++) {
> - PageTable[Index] = Address | IA32_PG_RW | IA32_PG_P;
> + PageTable[Index] = Address | PAGE_ATTRIBUTE_BITS;
> if (!IsAddressValid (Address, &Nx)) {
> - PageTable[Index] = PageTable[Index] & (INTN)(INT32)(~(IA32_PG_RW |
> IA32_PG_P));
> + PageTable[Index] = PageTable[Index] &
> (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
> }
> if (Nx && mXdSupported) {
> PageTable[Index] = PageTable[Index] | IA32_PG_NX;
> @@ -262,7 +262,7 @@ RestorePageTableAbove4G (
> //
> // Patch to remove present flag and rw flag.
> //
> - PageTable[PTIndex] = PageTable[PTIndex] & (INTN)(INT32)(~(IA32_PG_RW
> | IA32_PG_P));
> + PageTable[PTIndex] = PageTable[PTIndex] &
> (INTN)(INT32)(~PAGE_ATTRIBUTE_BITS);
> }
> //
> // Set XD bit to 1
> @@ -289,7 +289,7 @@ RestorePageTableAbove4G (
> //
> // Add present flag or clear XD flag to make page fault handler succeed.
> //
> - PageTable[PTIndex] |= (UINT64)(IA32_PG_RW | IA32_PG_P);
> + PageTable[PTIndex] |= (UINT64)(PAGE_ATTRIBUTE_BITS);
> if ((ErrorCode & IA32_PF_EC_ID) != 0) {
> //
> // If page fault is caused by instruction fetch, clear XD bit in the
> entry.
> --
> 1.9.5.msysgit.0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
