Re: [Patch V6 06/22] UefiCpuPkg/CpuPageTableLib: Fix issue when splitting leaf entry

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Friday, March 24, 2023 4:52 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V6 06/22] UefiCpuPkg/CpuPageTableLib: Fix issue when
> splitting leaf entry
> 
> When splitting leaf parent entry to smaller granularity, create
> child page table before modifing parent entry. In previous code
> logic, when splitting a leaf parent entry, parent entry will
> point to a null 4k memory before child page table is created in
> this 4k memory. When the page table to be modified is the page
> table in CR3, if the executed CpuPageTableLib code is in the
> range mapped by the modified leaf parent entry, then issue will
> happen.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 19
> +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index 57f1db203b..f09bb63ad1 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -363,21 +363,24 @@ PageTableLibMapInLevel (
>        //
>        // Create 512 child-level entries that map to 2M/4K.
>        //
> -      ParentPagingEntry->Uintn = (UINTN)Buffer + *BufferSize;
> -      ZeroMem ((VOID *)ParentPagingEntry->Uintn, SIZE_4KB);
> +      PagingEntry = (IA32_PAGING_ENTRY *)((UINTN)Buffer + *BufferSize);
> +      ZeroMem (PagingEntry, SIZE_4KB);
> +
> +      for (SubOffset = 0, Index = 0; Index < 512; Index++) {
> +        PagingEntry[Index].Uint64 = OneOfPagingEntry.Uint64 + SubOffset;
> +        SubOffset                += RegionLength;
> +      }
> 
>        //
>        // Set NOP attributes
>        // Note: Should NOT inherit the attributes from the original entry because
> a zero RW bit
>        //       will make the entire region read-only even the child entries set the
> RW bit.
>        //
> +      // Non-leaf entry doesn't have PAT bit. So use
> ~IA32_PE_BASE_ADDRESS_MASK_40 is to make sure PAT bit
> +      // (bit12) in original big-leaf entry is not assigned to
> PageTableBaseAddress field of non-leaf entry.
> +      //
>        PageTableLibSetPnle (&ParentPagingEntry->Pnle, &NopAttribute,
> &AllOneMask);
> -
> -      PagingEntry = (IA32_PAGING_ENTRY
> *)(UINTN)IA32_PNLE_PAGE_TABLE_BASE_ADDRESS (&ParentPagingEntry-
> >Pnle);
> -      for (SubOffset = 0, Index = 0; Index < 512; Index++) {
> -        PagingEntry[Index].Uint64 = OneOfPagingEntry.Uint64 + SubOffset;
> -        SubOffset                += RegionLength;
> -      }
> +      ParentPagingEntry->Uint64 = ((UINTN)(VOID *)PagingEntry) |
> (ParentPagingEntry->Uint64 & (~IA32_PE_BASE_ADDRESS_MASK_40));
>      }
>    } else {
>      //
> --
> 2.31.1.windows.1