Re: [edk2] [PATCH v3 1/2] MdeModulePkg/DxeIpl: Mark page table as read-only

2017-12-06 Thread Wang, Jian J
Hi,

> -Original Message-
> From: Ni, Ruiyu
> Sent: Wednesday, December 06, 2017 6:05 PM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Dong, Eric ;
> Zeng, Star 
> Subject: Re: [edk2] [PATCH v3 1/2] MdeModulePkg/DxeIpl: Mark page table as
> read-only
> 
> On 12/5/2017 4:16 PM, Jian J Wang wrote:
> >> v3:
> >> Remove the public definition of PAGE_TABLE_POOL_HEADER but keep
> similar
> >> concept locally. CpuDxe has its own page table pool.
> >
> >> v2:
> >> Introduce page table pool to ease the page table memory allocation and
> >> protection, which replaces the direct calling of AllocatePages().
> >
> > This patch will set the memory pages used for page table as read-only
> > memory after the paging is setup. CR0.WP must set to let it take into
> > effect.
> >
> > A simple page table memory management mechanism, page table pool
> concept,
> > is introduced to simplify the page table memory allocation and protection.
> > It will also help to reduce the potential recursive "split" action during
> > updating memory paging attributes.
> >
> > The basic idea is to allocate a bunch of continuous pages of memory in
> > advance as one or more page table pools, and all future page tables
> > consumption will happen in those pool instead of system memory. If the page
> > pool is reserved at the boundary of 2MB page and with same size of 2MB page,
> > there's no page granularity "split" operation will be needed, because the
> > memory of new page tables (if needed) will be usually in the same page as
> > target page table you're working on.
> >
> > And since we have centralized page tables (a few 2MB pages), it's easier
> > to protect them by changing their attributes to be read-only once and for
> > all. There's no need to apply the protection for new page tables any more
> > as long as the pool has free pages available.
> >
> > Once current page table pool has been used up, one can allocate another 2MB
> > memory pool and just set this new 2MB memory block to be read-only instead
> of
> > setting the new page tables one page by one page.
> >
> > Two new PCDs PcdPageTablePoolUnitSize and PcdPageTablePoolAlignment
> are used
> > to specify the size and alignment for page table pool. For IA32 processor
> > 0x20 (2MB) is the only choice for both of them to meet the requirement
> of
> > page table pool.
> >
> > Cc: Jiewen Yao 
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> > Cc: Ruiyu Ni 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang 
> > ---
> >   MdeModulePkg/Core/DxeIplPeim/DxeIpl.h|  34 +++
> >   MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |   8 +-
> >   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 301
> ++-
> >   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h |  26 ++
> >   4 files changed, 365 insertions(+), 4 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> > index f3aabdb7e0..9dc80b1508 100644
> > --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> > +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
> > @@ -265,4 +265,38 @@ IsNullDetectionEnabled (
> > VOID
> > );
> >
> > +/**
> > +  Prevent the memory pages used for page table from been overwritten.
> > +
> > +  @param[in] PageTableBaseBase address of page table (CR3).
> > +
> > +**/
> > +VOID
> > +EnablePageTableProtection (
> > +  IN  UINTN PageTableBase,
> > +  IN  BOOLEAN   Level4Paging
> > +  );
> > +
> > +/**
> > +  This API provides a way to allocate memory for page table.
> > +
> > +  This API can be called more than once to allocate memory for page tables.
> > +
> > +  Allocates the number of 4KB pages and returns a pointer to the allocated
> > +  buffer. The buffer returned is aligned on a 4KB boundary.
> > +
> > +  If Pages is 0, then NULL is returned.
> > +  If there is not enough memory remaining to satisfy the request, then 
> > NULL is
> > +  returned.
> > +
> > +  @param  Pages The number of 4 KB pages to allocate.
> > +
> > +  @return A pointer to the allocated buffer or NULL if allocation fails.
> > +
> > +**/
> > +VOID *
> > +AllocatePageTableMemory (
> > +  IN UINTN   Pages
> > +  );
> > +
> >   #endif
> > diff --git a/MdeModulePkg/Core/Dxe

Re: [edk2] [PATCH v3 1/2] MdeModulePkg/DxeIpl: Mark page table as read-only

2017-12-06 Thread Ni, Ruiyu

On 12/5/2017 4:16 PM, Jian J Wang wrote:

v3:
Remove the public definition of PAGE_TABLE_POOL_HEADER but keep similar
concept locally. CpuDxe has its own page table pool.



v2:
Introduce page table pool to ease the page table memory allocation and
protection, which replaces the direct calling of AllocatePages().


This patch will set the memory pages used for page table as read-only
memory after the paging is setup. CR0.WP must set to let it take into
effect.

A simple page table memory management mechanism, page table pool concept,
is introduced to simplify the page table memory allocation and protection.
It will also help to reduce the potential recursive "split" action during
updating memory paging attributes.

The basic idea is to allocate a bunch of continuous pages of memory in
advance as one or more page table pools, and all future page tables
consumption will happen in those pool instead of system memory. If the page
pool is reserved at the boundary of 2MB page and with same size of 2MB page,
there's no page granularity "split" operation will be needed, because the
memory of new page tables (if needed) will be usually in the same page as
target page table you're working on.

And since we have centralized page tables (a few 2MB pages), it's easier
to protect them by changing their attributes to be read-only once and for
all. There's no need to apply the protection for new page tables any more
as long as the pool has free pages available.

Once current page table pool has been used up, one can allocate another 2MB
memory pool and just set this new 2MB memory block to be read-only instead of
setting the new page tables one page by one page.

Two new PCDs PcdPageTablePoolUnitSize and PcdPageTablePoolAlignment are used
to specify the size and alignment for page table pool. For IA32 processor
0x20 (2MB) is the only choice for both of them to meet the requirement of
page table pool.

Cc: Jiewen Yao 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
  MdeModulePkg/Core/DxeIplPeim/DxeIpl.h|  34 +++
  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |   8 +-
  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 301 ++-
  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h |  26 ++
  4 files changed, 365 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h 
b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
index f3aabdb7e0..9dc80b1508 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
@@ -265,4 +265,38 @@ IsNullDetectionEnabled (
VOID
);
  
+/**

+  Prevent the memory pages used for page table from been overwritten.
+
+  @param[in] PageTableBaseBase address of page table (CR3).
+
+**/
+VOID
+EnablePageTableProtection (
+  IN  UINTN PageTableBase,
+  IN  BOOLEAN   Level4Paging
+  );
+
+/**
+  This API provides a way to allocate memory for page table.
+
+  This API can be called more than once to allocate memory for page tables.
+
+  Allocates the number of 4KB pages and returns a pointer to the allocated
+  buffer. The buffer returned is aligned on a 4KB boundary.
+
+  If Pages is 0, then NULL is returned.
+  If there is not enough memory remaining to satisfy the request, then NULL is
+  returned.
+
+  @param  Pages The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+AllocatePageTableMemory (
+  IN UINTN   Pages
+  );
+
  #endif
diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index 5649265367..13fff28e93 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -99,7 +99,7 @@ Create4GPageTablesIa32Pae (
NumberOfPdpEntriesNeeded = (UINT32) LShiftU64 (1, (PhysicalAddressBits - 
30));
  
TotalPagesNum = NumberOfPdpEntriesNeeded + 1;

-  PageAddress = (UINTN) AllocatePages (TotalPagesNum);
+  PageAddress = (UINTN) AllocatePageTableMemory (TotalPagesNum);
ASSERT (PageAddress != 0);
  
PageMap = (VOID *) PageAddress;

@@ -149,6 +149,12 @@ Create4GPageTablesIa32Pae (
);
}
  
+  //

+  // Protect the page table by marking the memory used for page table to be
+  // read-only.
+  //
+  EnablePageTableProtection ((UINTN)PageMap, FALSE);
+
return (UINTN) PageMap;
  }
  
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c

index 29b6205e88..4ef3521224 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -31,6 +31,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
  #include "DxeIpl.h"
  #include "VirtualMemory.h"
  
+//

+// Global variable to keep track current available memory used as page table.
+//
+

[edk2] [PATCH v3 1/2] MdeModulePkg/DxeIpl: Mark page table as read-only

2017-12-05 Thread Jian J Wang
> v3:
>Remove the public definition of PAGE_TABLE_POOL_HEADER but keep similar
>concept locally. CpuDxe has its own page table pool.

> v2:
>Introduce page table pool to ease the page table memory allocation and
>protection, which replaces the direct calling of AllocatePages().

This patch will set the memory pages used for page table as read-only
memory after the paging is setup. CR0.WP must set to let it take into
effect.

A simple page table memory management mechanism, page table pool concept,
is introduced to simplify the page table memory allocation and protection.
It will also help to reduce the potential recursive "split" action during
updating memory paging attributes.

The basic idea is to allocate a bunch of continuous pages of memory in
advance as one or more page table pools, and all future page tables
consumption will happen in those pool instead of system memory. If the page
pool is reserved at the boundary of 2MB page and with same size of 2MB page,
there's no page granularity "split" operation will be needed, because the
memory of new page tables (if needed) will be usually in the same page as
target page table you're working on.

And since we have centralized page tables (a few 2MB pages), it's easier
to protect them by changing their attributes to be read-only once and for
all. There's no need to apply the protection for new page tables any more
as long as the pool has free pages available.

Once current page table pool has been used up, one can allocate another 2MB
memory pool and just set this new 2MB memory block to be read-only instead of
setting the new page tables one page by one page.

Two new PCDs PcdPageTablePoolUnitSize and PcdPageTablePoolAlignment are used
to specify the size and alignment for page table pool. For IA32 processor
0x20 (2MB) is the only choice for both of them to meet the requirement of
page table pool.

Cc: Jiewen Yao 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.h|  34 +++
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |   8 +-
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 301 ++-
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h |  26 ++
 4 files changed, 365 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h 
b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
index f3aabdb7e0..9dc80b1508 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
@@ -265,4 +265,38 @@ IsNullDetectionEnabled (
   VOID
   );
 
+/**
+  Prevent the memory pages used for page table from been overwritten.
+
+  @param[in] PageTableBaseBase address of page table (CR3).
+
+**/
+VOID
+EnablePageTableProtection (
+  IN  UINTN PageTableBase,
+  IN  BOOLEAN   Level4Paging
+  );
+
+/**
+  This API provides a way to allocate memory for page table.
+
+  This API can be called more than once to allocate memory for page tables.
+
+  Allocates the number of 4KB pages and returns a pointer to the allocated
+  buffer. The buffer returned is aligned on a 4KB boundary.
+
+  If Pages is 0, then NULL is returned.
+  If there is not enough memory remaining to satisfy the request, then NULL is
+  returned.
+
+  @param  Pages The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+AllocatePageTableMemory (
+  IN UINTN   Pages
+  );
+
 #endif
diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index 5649265367..13fff28e93 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -99,7 +99,7 @@ Create4GPageTablesIa32Pae (
   NumberOfPdpEntriesNeeded = (UINT32) LShiftU64 (1, (PhysicalAddressBits - 
30));
 
   TotalPagesNum = NumberOfPdpEntriesNeeded + 1;
-  PageAddress = (UINTN) AllocatePages (TotalPagesNum);
+  PageAddress = (UINTN) AllocatePageTableMemory (TotalPagesNum);
   ASSERT (PageAddress != 0);
 
   PageMap = (VOID *) PageAddress;
@@ -149,6 +149,12 @@ Create4GPageTablesIa32Pae (
   );
   }
 
+  //
+  // Protect the page table by marking the memory used for page table to be
+  // read-only.
+  //
+  EnablePageTableProtection ((UINTN)PageMap, FALSE);
+
   return (UINTN) PageMap;
 }
 
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c 
b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 29b6205e88..4ef3521224 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -31,6 +31,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include "DxeIpl.h"
 #include "VirtualMemory.h"
 
+//
+// Global variable to keep track current available memory used as page table.
+//
+PAGE_TABLE_POOL   *mPageTablePool = NULL;
+
 /**
   Clear legacy me