Re: [PATCH 3/6] UefiCpuPkg/CpuDxe: clear NX attr for page directory

["Dong, Eric" <eric.dong@intel.com>]

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jian J Wang
> Sent: Monday, January 15, 2018 4:55 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2] [PATCH 3/6] UefiCpuPkg/CpuDxe: clear NX attr for page
> directory
> 
> If PcdDxeNxMemoryProtectionPolicy is set to enable protection for memory
> of EfiBootServicesCode, EfiConventionalMemory and
> EfiReservedMemoryType, the BIOS will hang at a page fault exception
> randomly.
> 
> The root cause is that the memory allocation for driver images (actually a
> memory type conversion from free memory, type of EfiConventionalMemory,
> to code memory, type of EfiBootServicesCode/EfiRuntimeServicesCode)
> will get memory with NX set, because the CpuDxe driver will keep the NX
> attribute (with free memory) in page directory during page table splitting and
> then override the NX attribute of all its entries.
> 
> This patch fixes this issue by not inheriting NX attribute when turning a page
> entry into a page directory during page granularity split.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index a9c9bc9d5e..1654e71103 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -528,7 +528,7 @@ SplitPage (
>        for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
>          NewPageEntry[Index] = (BaseAddress + SIZE_4KB * Index) |
> AddressEncMask | ((*PageEntry) & PAGE_PROGATE_BITS);
>        }
> -      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> ((*PageEntry) & PAGE_PROGATE_BITS);
> +      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> + ((*PageEntry) & PAGE_ATTRIBUTE_BITS);
>        return RETURN_SUCCESS;
>      } else {
>        return RETURN_UNSUPPORTED;
> @@ -549,7 +549,7 @@ SplitPage (
>        for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
>          NewPageEntry[Index] = (BaseAddress + SIZE_2MB * Index) |
> AddressEncMask | IA32_PG_PS | ((*PageEntry) & PAGE_PROGATE_BITS);
>        }
> -      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> ((*PageEntry) & PAGE_PROGATE_BITS);
> +      (*PageEntry) = (UINT64)(UINTN)NewPageEntry | AddressEncMask |
> + ((*PageEntry) & PAGE_ATTRIBUTE_BITS);
>        return RETURN_SUCCESS;
>      } else {
>        return RETURN_UNSUPPORTED;
> @@ -979,7 +979,7 @@ RefreshGcdMemoryAttributesFromPaging (
>                          );
>          ASSERT_EFI_ERROR (Status);
>          DEBUG ((
> -          DEBUG_INFO,
> +          DEBUG_VERBOSE,
>            "Updated memory space attribute: [%lu] %016lx - %016lx (%016lx -
> > %016lx)\r\n",
>            (UINT64)Index, BaseAddress, BaseAddress + Length - 1,
>            MemorySpaceMap[Index].Attributes,
> --
> 2.15.1.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel