From: "Ni, Ray" <ray.ni@intel.com>
To: Ard Biesheuvel <ardb@kernel.org>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"Wu, Jiaxin" <jiaxin.wu@intel.com>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>,
Gerd Hoffmann <kraxel@redhat.com>,
Taylor Beebe <t@taylorbeebe.com>,
Oliver Smith-Denny <osd@smith-denny.com>,
"Bi, Dandan" <dandan.bi@intel.com>,
"Tan, Dun" <dun.tan@intel.com>,
"Gao, Liming" <gaoliming@byosoft.com.cn>,
"Kinney, Michael D" <michael.d.kinney@intel.com>,
Michael Kubacki <mikuback@linux.microsoft.com>,
"Dong, Eric" <eric.dong@intel.com>,
"Kumar, Rahul R" <rahul.r.kumar@intel.com>,
Kun Qin <kuqin12@gmail.com>
Subject: Re: [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM
Date: Fri, 9 Jun 2023 00:24:32 +0000 [thread overview]
Message-ID: <MN6PR11MB8244EFBC6EA7DA15160BCBD48C51A@MN6PR11MB8244.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230608172323.9096-3-ardb@kernel.org>
Ard,
https://github.com/tianocore/edk2/blob/8314a85893f5b75baa0031a5138028188a626243/UefiCpuPkg/SecCore/SecMain.c#L637 is changed by @Wu, Jiaxin to create page table in permanent memory.
(I didn't check your patch in detail. But it sounds like you try to do the same thing that above code has already done.)
Thanks,
Ray
> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Friday, June 9, 2023 1:23 AM
> To: devel@edk2.groups.io
> Cc: Ard Biesheuvel <ardb@kernel.org>; Ni, Ray <ray.ni@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor
> Beebe <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>;
> Bi, Dandan <dandan.bi@intel.com>; Tan, Dun <dun.tan@intel.com>; Gao,
> Liming <gaoliming@byosoft.com.cn>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Michael Kubacki
> <mikuback@linux.microsoft.com>; Dong, Eric <eric.dong@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Kun Qin <kuqin12@gmail.com>
> Subject: [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in
> permanent DRAM
>
> Currently, we rely on the logic in DXE IPL to create new page tables
> from scratch when executing in X64 mode, which means that we run with
> the initial page tables all throughout PEI, and never enable protections
> such as the CPU stack guard, even though the logic is already in place
> for IA32.
>
> So let's enable the existing logic for X64 as well. This will permit us
> to apply stricter memory permissions to code and data allocations, as
> well as the stack, when executing in PEI. It also makes the DxeIpl logic
> redundant, and should allow us to make the PcdDxeIplBuildPageTables
> feature PCD limited to IA32 DxeIpl loading the x64 DXE core.
>
> When running in long mode, use the same logic that DxeIpl uses to
> determine the size of the address space, whether or not to use 1 GB leaf
> entries and whether or not to use 5 level paging. Note that in long
> mode, PEI is entered with paging enabled, and given that switching
> between 4 and 5 levels of paging is not currently supported without
> dropping out of 64-bit mode temporarily, all we can do is carry on
> without changing the number of levels.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 2 +
> UefiCpuPkg/CpuMpPei/CpuPaging.c | 163 ++++++++++++++++----
> 2 files changed, 139 insertions(+), 26 deletions(-)
>
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> index 865be5627e8551ee..77eecaa0ea035b38 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> @@ -65,6 +65,8 @@ [Ppis]
> [Pcd]
>
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMas
> k ## CONSUMES
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ##
> CONSUMES
>
> + gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ##
> SOMETIMES_CONSUMES
>
> + gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable ##
> SOMETIMES_CONSUMES
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList ##
> SOMETIMES_CONSUMES
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize ##
> SOMETIMES_CONSUMES
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ##
> SOMETIMES_CONSUMES
>
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index 175e47ccd737a0c1..2a901e44253434c2 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -302,7 +302,7 @@ ConvertMemoryPageAttributes (
> return RETURN_INVALID_PARAMETER;
>
> }
>
>
>
> - MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_UINT32;
>
> + MaximumAddress = (EFI_PHYSICAL_ADDRESS)MAX_ADDRESS;
>
> if ((BaseAddress > MaximumAddress) ||
>
> (Length > MaximumAddress) ||
>
> (BaseAddress > MaximumAddress - (Length - 1)))
>
> @@ -350,16 +350,91 @@ ConvertMemoryPageAttributes (
> return RETURN_SUCCESS;
>
> }
>
>
>
> +/*
>
> + * Get physical address bits supported.
>
> + *
>
> + * @return The number of supported physical address bits.
>
> + */
>
> +STATIC
>
> +UINT8
>
> +GetPhysicalAddressBits (
>
> + VOID
>
> + )
>
> +{
>
> + EFI_HOB_CPU *Hob;
>
> + UINT32 RegEax;
>
> +
>
> + Hob = GetFirstHob (EFI_HOB_TYPE_CPU);
>
> + if (Hob != NULL) {
>
> + return Hob->SizeOfMemorySpace;
>
> + }
>
> +
>
> + AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
>
> + if (RegEax >= 0x80000008) {
>
> + AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);
>
> + return (UINT8)RegEax;
>
> + }
>
> +
>
> + return 36;
>
> +}
>
> +
>
> +/*
>
> + * Determine and return the paging mode to be used in long mode, based on
> PCD
>
> + * configuration and CPU support for 1G leaf descriptors and 5 level paging.
>
> + *
>
> + * @return The paging mode
>
> + */
>
> +STATIC
>
> +PAGING_MODE
>
> +GetPagingMode (
>
> + VOID
>
> + )
>
> +{
>
> + BOOLEAN Page5LevelSupport;
>
> + BOOLEAN Page1GSupport;
>
> + UINT32 RegEax;
>
> + UINT32 RegEdx;
>
> + IA32_CR4 Cr4;
>
> +
>
> + Cr4.UintN = AsmReadCr4 ();
>
> + Page5LevelSupport = (Cr4.Bits.LA57 != 0);
>
> + ASSERT (PcdGetBool (PcdUse5LevelPageTable) == Page5LevelSupport);
>
> +
>
> + Page1GSupport = FALSE;
>
> + if (PcdGetBool (PcdUse1GPageTable)) {
>
> + AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
>
> + if (RegEax >= 0x80000001) {
>
> + AsmCpuid (0x80000001, NULL, NULL, NULL, &RegEdx);
>
> + if ((RegEdx & BIT26) != 0) {
>
> + Page1GSupport = TRUE;
>
> + }
>
> + }
>
> + }
>
> +
>
> + if (Page5LevelSupport && Page1GSupport) {
>
> + return Paging5Level1GB;
>
> + } else if (Page5LevelSupport) {
>
> + return Paging5Level;
>
> + } else if (Page1GSupport) {
>
> + return Paging4Level1GB;
>
> + } else {
>
> + return Paging4Level;
>
> + }
>
> +}
>
> +
>
> /**
>
> - Enable PAE Page Table.
>
> + Enable Page Table.
>
>
>
> - @retval EFI_SUCCESS The PAE Page Table was enabled successfully.
>
> - @retval EFI_OUT_OF_RESOURCES The PAE Page Table could not be enabled
> due to lack of available memory.
>
> + @param LongMode Whether the execution mode is 64 bit
>
> +
>
> + @retval EFI_SUCCESS The Page Table was enabled successfully.
>
> + @retval EFI_OUT_OF_RESOURCES The Page Table could not be enabled
> due to lack of available memory.
>
>
>
> **/
>
> +STATIC
>
> EFI_STATUS
>
> -EnablePaePageTable (
>
> - VOID
>
> +EnablePageTable (
>
> + IN BOOLEAN LongMode
>
> )
>
> {
>
> EFI_STATUS Status;
>
> @@ -369,6 +444,8 @@ EnablePaePageTable (
> UINTN BufferSize;
>
> IA32_MAP_ATTRIBUTE MapAttribute;
>
> IA32_MAP_ATTRIBUTE MapMask;
>
> + PAGING_MODE PagingMode;
>
> + UINT64 Length;
>
>
>
> PageTable = 0;
>
> Buffer = NULL;
>
> @@ -378,10 +455,28 @@ EnablePaePageTable (
> MapAttribute.Bits.Present = 1;
>
> MapAttribute.Bits.ReadWrite = 1;
>
>
>
> - //
>
> - // 1:1 map 4GB in 32bit mode
>
> - //
>
> - Status = PageTableMap (&PageTable, PagingPae, 0, &BufferSize, 0, SIZE_4GB,
> &MapAttribute, &MapMask, NULL);
>
> + if (!LongMode) {
>
> + //
>
> + // 1:1 map 4GB in 32bit mode
>
> + //
>
> + PagingMode = PagingPae;
>
> + Length = SIZE_4GB;
>
> + } else {
>
> + PagingMode = GetPagingMode ();
>
> + Length = LShiftU64 (1, GetPhysicalAddressBits ());
>
> + }
>
> +
>
> + Status = PageTableMap (
>
> + &PageTable,
>
> + PagingMode,
>
> + 0,
>
> + &BufferSize,
>
> + 0,
>
> + Length,
>
> + &MapAttribute,
>
> + &MapMask,
>
> + NULL
>
> + );
>
> ASSERT (Status == EFI_BUFFER_TOO_SMALL);
>
> if (Status != EFI_BUFFER_TOO_SMALL) {
>
> return Status;
>
> @@ -398,12 +493,23 @@ EnablePaePageTable (
>
>
> DEBUG ((
>
> DEBUG_INFO,
>
> - "EnablePaePageTable: Created PageTable = 0x%x, BufferSize = %x\n",
>
> + "%a: Created PageTable = 0x%x, BufferSize = %x\n",
>
> + __func__,
>
> PageTable,
>
> BufferSize
>
> ));
>
>
>
> - Status = PageTableMap (&PageTable, PagingPae, Buffer, &BufferSize, 0,
> SIZE_4GB, &MapAttribute, &MapMask, NULL);
>
> + Status = PageTableMap (
>
> + &PageTable,
>
> + PagingMode,
>
> + Buffer,
>
> + &BufferSize,
>
> + 0,
>
> + Length,
>
> + &MapAttribute,
>
> + &MapMask,
>
> + NULL
>
> + );
>
> ASSERT_EFI_ERROR (Status);
>
> if (EFI_ERROR (Status) || (PageTable == 0)) {
>
> return EFI_OUT_OF_RESOURCES;
>
> @@ -414,15 +520,17 @@ EnablePaePageTable (
> //
>
> AsmWriteCr3 (PageTable);
>
>
>
> - //
>
> - // Enable CR4.PAE
>
> - //
>
> - AsmWriteCr4 (AsmReadCr4 () | BIT5);
>
> + if (!LongMode) {
>
> + //
>
> + // Enable CR4.PAE
>
> + //
>
> + AsmWriteCr4 (AsmReadCr4 () | BIT5);
>
>
>
> - //
>
> - // Enable CR0.PG
>
> - //
>
> - AsmWriteCr0 (AsmReadCr0 () | BIT31);
>
> + //
>
> + // Enable CR0.PG
>
> + //
>
> + AsmWriteCr0 (AsmReadCr0 () | BIT31);
>
> + }
>
>
>
> return Status;
>
> }
>
> @@ -557,6 +665,9 @@ MemoryDiscoveredPpiNotifyCallback (
> EDKII_MIGRATED_FV_INFO *MigratedFvInfo;
>
> EFI_PEI_HOB_POINTERS Hob;
>
> IA32_CR0 Cr0;
>
> + BOOLEAN LongMode;
>
> +
>
> + LongMode = (sizeof (UINTN) == sizeof (UINT64));
>
>
>
> //
>
> // Paging must be setup first. Otherwise the exception TSS setup during MP
>
> @@ -565,7 +676,7 @@ MemoryDiscoveredPpiNotifyCallback (
> //
>
> InitStackGuard = FALSE;
>
> Hob.Raw = NULL;
>
> - if (IsIa32PaeSupported ()) {
>
> + if (LongMode || IsIa32PaeSupported ()) {
>
> Hob.Raw = GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid);
>
> InitStackGuard = PcdGetBool (PcdCpuStackGuard);
>
> }
>
> @@ -575,12 +686,12 @@ MemoryDiscoveredPpiNotifyCallback (
> // is to enable paging if it is not enabled (only in 32bit mode).
>
> //
>
> Cr0.UintN = AsmReadCr0 ();
>
> - if ((Cr0.Bits.PG == 0) && (InitStackGuard || (Hob.Raw != NULL))) {
>
> - ASSERT (sizeof (UINTN) == sizeof (UINT32));
>
> -
>
> - Status = EnablePaePageTable ();
>
> + if (LongMode ||
>
> + ((Cr0.Bits.PG == 0) && (InitStackGuard || (Hob.Raw != NULL))))
>
> + {
>
> + Status = EnablePageTable (LongMode);
>
> if (EFI_ERROR (Status)) {
>
> - DEBUG ((DEBUG_ERROR, "MemoryDiscoveredPpiNotifyCallback: Failed to
> enable PAE page table: %r.\n", Status));
>
> + DEBUG ((DEBUG_ERROR, "%a: Failed to enable page table: %r.\n",
> __func__, Status));
>
> CpuDeadLoop ();
>
> }
>
> }
>
> --
> 2.39.2
prev parent reply other threads:[~2023-06-09 0:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-08 17:23 [PATCH 0/2] UefiCpuPkg/CpuMpPei X64: reallocate page tables in PEI Ard Biesheuvel
2023-06-08 17:23 ` [PATCH 1/2] UefiCpuPkg/CpuMpPei: Print correct buffer size used for page table Ard Biesheuvel
2023-06-08 19:25 ` [edk2-devel] " Michael Kubacki
2023-06-08 17:23 ` [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM Ard Biesheuvel
2023-06-08 17:39 ` [edk2-devel] " Oliver Smith-Denny
2023-06-08 17:48 ` Ard Biesheuvel
2023-06-08 18:27 ` Sean
2023-06-08 19:32 ` Michael Kubacki
2023-06-08 22:17 ` Ard Biesheuvel
2023-06-08 19:38 ` Michael Kubacki
2023-06-09 0:24 ` Ni, Ray [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN6PR11MB8244EFBC6EA7DA15160BCBD48C51A@MN6PR11MB8244.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox