> -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > Lendacky, Thomas > Sent: Friday, July 31, 2020 2:43 AM > To: devel@edk2.groups.io > Cc: Brijesh Singh <brijesh.si...@amd.com>; Ard Biesheuvel > <ard.biesheu...@arm.com>; Dong, Eric <eric.d...@intel.com>; Justen, > Jordan L <jordan.l.jus...@intel.com>; Laszlo Ersek <ler...@redhat.com>; > Gao, Liming <liming....@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Ni, Ray <ray...@intel.com>; Wang, Jian J > <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; Bi, Dandan > <dandan...@intel.com> > Subject: [edk2-devel] [PATCH v13 05/46] MdeModulePkg/DxeIplPeim: > Support GHCB pages when creating page tables > > From: Tom Lendacky <thomas.lenda...@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.w...@intel.com> > Cc: Hao A Wu <hao.a...@intel.com> > Cc: Dandan Bi <dandan...@intel.com> > Cc: Liming Gao <liming....@intel.com> > Signed-off-by: Tom Lendacky <thomas.lenda...@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...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-