[edk2-devel] [PATCH v13 05/46] MdeModulePkg/DxeIplPeim: Support GHCB pages when creating page tables
Wu, Hao A
hao.a.wu at intel.com
Mon Aug 3 05:41:08 UTC 2020
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> Lendacky, Thomas
> Sent: Friday, July 31, 2020 2:43 AM
> To: devel at edk2.groups.io
> Cc: Brijesh Singh <brijesh.singh at amd.com>; Ard Biesheuvel
> <ard.biesheuvel at arm.com>; Dong, Eric <eric.dong at intel.com>; Justen,
> Jordan L <jordan.l.justen at intel.com>; Laszlo Ersek <lersek at redhat.com>;
> Gao, Liming <liming.gao at intel.com>; Kinney, Michael D
> <michael.d.kinney at intel.com>; Ni, Ray <ray.ni at intel.com>; Wang, Jian J
> <jian.j.wang at intel.com>; Wu, Hao A <hao.a.wu at intel.com>; Bi, Dandan
> <dandan.bi at intel.com>
> Subject: [edk2-devel] [PATCH v13 05/46] MdeModulePkg/DxeIplPeim:
> Support GHCB pages when creating page tables
>
> From: Tom Lendacky <thomas.lendacky at amd.com>
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
>
> GHCB pages must be mapped as shared pages, so modify the process of
> creating identity mapped pagetable entries so that GHCB entries are created
> without the encryption bit set. The GHCB range consists of two pages per
> CPU, the first being the GHCB and the second being a per-CPU variable page.
> Only the GHCB page is mapped as shared.
>
> Cc: Jian J Wang <jian.j.wang at intel.com>
> Cc: Hao A Wu <hao.a.wu at intel.com>
> Cc: Dandan Bi <dandan.bi at intel.com>
> Cc: Liming Gao <liming.gao at intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky at amd.com>
> ---
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 +
> .../Core/DxeIplPeim/X64/VirtualMemory.h | 12 +++-
> .../Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 +-
> .../Core/DxeIplPeim/X64/DxeLoadFunc.c | 11 +++-
> .../Core/DxeIplPeim/X64/VirtualMemory.c | 57 +++++++++++++++----
> 5 files changed, 70 insertions(+), 16 deletions(-)
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index 3f1702854660..19b8a4c8aefa 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -115,6 +115,8 @@ [Pcd.IA32,Pcd.X64]
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ##
> CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable ##
> SOMETIMES_CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ##
> CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbSize ##
> CONSUMES
>
> [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
> gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ##
> SOMETIMES_CONSUMES
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
> b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
> index 2d0493f109e8..6b7c38a441d6 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
> @@ -201,6 +201,8 @@ EnableExecuteDisableBit (
> @param[in, out] PageEntry2M Pointer to 2M page entry.
> @param[in] StackBase Stack base address.
> @param[in] StackSize Stack size.
> + @param[in] GhcbBase GHCB page area base address.
> + @param[in] GhcbSize GHCB page area size.
>
> **/
> VOID
> @@ -208,7 +210,9 @@ Split2MPageTo4K (
> IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> IN OUT UINT64 *PageEntry2M,
> IN EFI_PHYSICAL_ADDRESS StackBase,
> - IN UINTN StackSize
> + IN UINTN StackSize,
> + IN EFI_PHYSICAL_ADDRESS GhcbBase,
> + IN UINTN GhcbSize
> );
>
> /**
> @@ -217,6 +221,8 @@ Split2MPageTo4K (
>
> @param[in] StackBase Stack base address.
> @param[in] StackSize Stack size.
> + @param[in] GhcbBase GHCB page area base address.
> + @param[in] GhcbSize GHCB page area size.
>
> @return The address of 4 level page map.
>
> @@ -224,7 +230,9 @@ Split2MPageTo4K (
> UINTN
> CreateIdentityMappingPageTables (
> IN EFI_PHYSICAL_ADDRESS StackBase,
> - IN UINTN StackSize
> + IN UINTN StackSize,
> + IN EFI_PHYSICAL_ADDRESS GhcbBase,
> + IN UINTN GhcbkSize
> );
>
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> index 6e8ca824d469..284b34818ca7 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
> @@ -123,7 +123,7 @@ Create4GPageTablesIa32Pae (
> //
> // Need to split this 2M page that covers stack range.
> //
> - Split2MPageTo4K (PhysicalAddress, (UINT64 *) PageDirectoryEntry,
> StackBase, StackSize);
> + Split2MPageTo4K (PhysicalAddress, (UINT64 *)
> + PageDirectoryEntry, StackBase, StackSize, 0, 0);
> } else {
> //
> // Fill in the Page Directory entries @@ -282,7 +282,7 @@
> HandOffToDxeCore (
> //
> // Create page table and save PageMapLevel4 to CR3
> //
> - PageTables = CreateIdentityMappingPageTables (BaseOfStack,
> STACK_SIZE);
> + PageTables = CreateIdentityMappingPageTables (BaseOfStack,
> + STACK_SIZE, 0, 0);
>
> //
> // End of PEI phase signal
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
> b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
> index f465eb1d8ac4..156a477d8467 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
> @@ -35,6 +35,8 @@ HandOffToDxeCore (
> UINT32 Index;
> EFI_VECTOR_HANDOFF_INFO *VectorInfo;
> EFI_PEI_VECTOR_HANDOFF_INFO_PPI *VectorHandoffInfoPpi;
> + VOID *GhcbBase;
> + UINTN GhcbSize;
>
> //
> // Clear page 0 and mark it as allocated if NULL pointer detection is
> enabled.
> @@ -81,12 +83,19 @@ HandOffToDxeCore (
> TopOfStack = (VOID *) ((UINTN) BaseOfStack + EFI_SIZE_TO_PAGES
> (STACK_SIZE) * EFI_PAGE_SIZE - CPU_STACK_ALIGNMENT);
> TopOfStack = ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);
>
> + //
> + // Get the address and size of the GHCB pages // GhcbBase = (VOID
> + *) PcdGet64 (PcdGhcbBase); GhcbSize = PcdGet64 (PcdGhcbSize);
> +
> PageTables = 0;
> if (FeaturePcdGet (PcdDxeIplBuildPageTables)) {
> //
> // Create page table and save PageMapLevel4 to CR3
> //
> - PageTables = CreateIdentityMappingPageTables
> ((EFI_PHYSICAL_ADDRESS) (UINTN) BaseOfStack, STACK_SIZE);
> + PageTables = CreateIdentityMappingPageTables
> ((EFI_PHYSICAL_ADDRESS) (UINTN) BaseOfStack, STACK_SIZE,
> +
> + (EFI_PHYSICAL_ADDRESS) (UINTN) GhcbBase, GhcbSize);
> } else {
> //
> // Set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> index 516cf908bc88..6831946c54d3 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> @@ -181,6 +181,8 @@ EnableExecuteDisableBit (
> @param Size Size of the given physical memory.
> @param StackBase Base address of stack.
> @param StackSize Size of stack.
> + @param GhcbBase Base address of GHCB pages.
> + @param GhcbSize Size of GHCB area.
>
> @retval TRUE Page table should be split.
> @retval FALSE Page table should not be split.
> @@ -190,7 +192,9 @@ ToSplitPageTable (
> IN EFI_PHYSICAL_ADDRESS Address,
> IN UINTN Size,
> IN EFI_PHYSICAL_ADDRESS StackBase,
> - IN UINTN StackSize
> + IN UINTN StackSize,
> + IN EFI_PHYSICAL_ADDRESS GhcbBase,
> + IN UINTN GhcbSize
> )
> {
> if (IsNullDetectionEnabled () && Address == 0) { @@ -209,6 +213,12 @@
> ToSplitPageTable (
> }
> }
>
> + if (GhcbBase != 0) {
> + if ((Address < GhcbBase + GhcbSize) && ((Address + Size) > GhcbBase)) {
> + return TRUE;
> + }
> + }
> +
> return FALSE;
> }
> /**
> @@ -322,6 +332,8 @@ AllocatePageTableMemory (
> @param[in, out] PageEntry2M Pointer to 2M page entry.
> @param[in] StackBase Stack base address.
> @param[in] StackSize Stack size.
> + @param[in] GhcbBase GHCB page area base address.
> + @param[in] GhcbSize GHCB page area size.
>
> **/
> VOID
> @@ -329,7 +341,9 @@ Split2MPageTo4K (
> IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> IN OUT UINT64 *PageEntry2M,
> IN EFI_PHYSICAL_ADDRESS StackBase,
> - IN UINTN StackSize
> + IN UINTN StackSize,
> + IN EFI_PHYSICAL_ADDRESS GhcbBase,
> + IN UINTN GhcbSize
> )
> {
> EFI_PHYSICAL_ADDRESS PhysicalAddress4K;
> @@ -355,7 +369,20 @@ Split2MPageTo4K (
> //
> // Fill in the Page Table entries
> //
> - PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K | AddressEncMask;
> + PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K;
> +
> + //
> + // The GHCB range consists of two pages per CPU, the GHCB and a
> + // per-CPU variable page. The GHCB page needs to be mapped as an
> + // unencrypted page while the per-CPU variable page needs to be
> + // mapped encrypted. These pages alternate in assignment.
> + //
> + if ((GhcbBase == 0)
> + || (PhysicalAddress4K < GhcbBase)
> + || (PhysicalAddress4K >= GhcbBase + GhcbSize)
> + || (((PhysicalAddress4K - GhcbBase) & SIZE_4KB) != 0)) {
> + PageTableEntry->Uint64 |= AddressEncMask;
> + }
> PageTableEntry->Bits.ReadWrite = 1;
>
> if ((IsNullDetectionEnabled () && PhysicalAddress4K == 0) || @@ -383,6
> +410,8 @@ Split2MPageTo4K (
> @param[in, out] PageEntry1G Pointer to 1G page entry.
> @param[in] StackBase Stack base address.
> @param[in] StackSize Stack size.
> + @param[in] GhcbBase GHCB page area base address.
> + @param[in] GhcbSize GHCB page area size.
>
> **/
> VOID
> @@ -390,7 +419,9 @@ Split1GPageTo2M (
> IN EFI_PHYSICAL_ADDRESS PhysicalAddress,
> IN OUT UINT64 *PageEntry1G,
> IN EFI_PHYSICAL_ADDRESS StackBase,
> - IN UINTN StackSize
> + IN UINTN StackSize,
> + IN EFI_PHYSICAL_ADDRESS GhcbBase,
> + IN UINTN GhcbSize
> )
> {
> EFI_PHYSICAL_ADDRESS PhysicalAddress2M;
> @@ -413,11 +444,11 @@ Split1GPageTo2M (
>
> PhysicalAddress2M = PhysicalAddress;
> for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512;
> IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M
> += SIZE_2MB) {
> - if (ToSplitPageTable (PhysicalAddress2M, SIZE_2MB, StackBase, StackSize))
> {
> + if (ToSplitPageTable (PhysicalAddress2M, SIZE_2MB, StackBase,
> + StackSize, GhcbBase, GhcbSize)) {
> //
> // Need to split this 2M page that covers NULL or stack range.
> //
> - Split2MPageTo4K (PhysicalAddress2M, (UINT64 *) PageDirectoryEntry,
> StackBase, StackSize);
> + Split2MPageTo4K (PhysicalAddress2M, (UINT64 *)
> + PageDirectoryEntry, StackBase, StackSize, GhcbBase, GhcbSize);
> } else {
> //
> // Fill in the Page Directory entries @@ -616,6 +647,8 @@
> EnablePageTableProtection (
>
> @param[in] StackBase Stack base address.
> @param[in] StackSize Stack size.
> + @param[in] GhcbBase GHCB base address.
> + @param[in] GhcbSize GHCB size.
>
> @return The address of 4 level page map.
>
> @@ -623,7 +656,9 @@ EnablePageTableProtection ( UINTN
> CreateIdentityMappingPageTables (
> IN EFI_PHYSICAL_ADDRESS StackBase,
> - IN UINTN StackSize
> + IN UINTN StackSize,
> + IN EFI_PHYSICAL_ADDRESS GhcbBase,
> + IN UINTN GhcbSize
> )
> {
> UINT32 RegEax;
> @@ -809,8 +844,8 @@ CreateIdentityMappingPageTables (
> PageDirectory1GEntry = (VOID *) PageDirectoryPointerEntry;
>
> for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries <
> 512; IndexOfPageDirectoryEntries++, PageDirectory1GEntry++, PageAddress
> += SIZE_1GB) {
> - if (ToSplitPageTable (PageAddress, SIZE_1GB, StackBase, StackSize)) {
> - Split1GPageTo2M (PageAddress, (UINT64 *) PageDirectory1GEntry,
> StackBase, StackSize);
> + if (ToSplitPageTable (PageAddress, SIZE_1GB, StackBase, StackSize,
> GhcbBase, GhcbSize)) {
> + Split1GPageTo2M (PageAddress, (UINT64 *)
> + PageDirectory1GEntry, StackBase, StackSize, GhcbBase, GhcbSize);
> } else {
> //
> // Fill in the Page Directory entries @@ -840,11 +875,11 @@
> CreateIdentityMappingPageTables (
> PageDirectoryPointerEntry->Bits.Present = 1;
>
> for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries <
> 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PageAddress +=
> SIZE_2MB) {
> - if (ToSplitPageTable (PageAddress, SIZE_2MB, StackBase, StackSize)) {
> + if (ToSplitPageTable (PageAddress, SIZE_2MB, StackBase,
> + StackSize, GhcbBase, GhcbSize)) {
> //
> // Need to split this 2M page that covers NULL or stack range.
> //
> - Split2MPageTo4K (PageAddress, (UINT64 *) PageDirectoryEntry,
> StackBase, StackSize);
> + Split2MPageTo4K (PageAddress, (UINT64 *)
> + PageDirectoryEntry, StackBase, StackSize, GhcbBase, GhcbSize);
Looks to me that the logic remains the same when PcdGhcbBase and PcdGhcbSize
are of their default values. Therefore:
Acked-by: Hao A Wu <hao.a.wu at intel.com>
Best Regards,
Hao Wu
> } else {
> //
> // Fill in the Page Directory entries
> --
> 2.27.0
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#63650): https://edk2.groups.io/g/devel/message/63650
Mute This Topic: https://groups.io/mt/75892685/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list