From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.3677.1686253132072140406 for ; Thu, 08 Jun 2023 12:38:52 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=IePBO4lE; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id 1232E20C145B; Thu, 8 Jun 2023 12:38:49 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 1232E20C145B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1686253131; bh=vF1U68PLKkmAJL9EqArgL2MuXDDB46hbqTLvjN39h2A=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=IePBO4lEFphd1lSoU2mnt6NCEy1II34/dkdlYIWCxVnDAeA49vBTfOrfWHxFBuNXf fFWhJjSkSw+FuDDIfgByBa9kLLvyXxXBNeX49Q4x/HWDJucLkjhjZbvkpRrMT9t15J 3xacLLjAHZqaSTlJvZO4pFsW+9Zo7dBNwEx8+Q08= Message-ID: <0b8890a6-04c5-b6a7-1c41-d3f13ccf6b2b@linux.microsoft.com> Date: Thu, 8 Jun 2023 15:38:48 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.2 Subject: Re: [edk2-devel] [PATCH 2/2] UefiCpuPkg/CpuMpPei X64: Reallocate page tables in permanent DRAM To: devel@edk2.groups.io, ardb@kernel.org Cc: Ray Ni , Jiewen Yao , Gerd Hoffmann , Taylor Beebe , Oliver Smith-Denny , Dandan Bi , Dun Tan , Liming Gao , "Kinney, Michael D" , Eric Dong , Rahul Kumar , Kun Qin References: <20230608172323.9096-1-ardb@kernel.org> <20230608172323.9096-3-ardb@kernel.org> From: "Michael Kubacki" In-Reply-To: <20230608172323.9096-3-ardb@kernel.org> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 6/8/2023 1:23 PM, Ard Biesheuvel wrote: > 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 > --- > 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.PcdPteMemoryEncryptionAddressOrMask ## 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; > > +} > > + I saw a similar patch come through earlier today adding GetMaxPlatformAddressBits() to CpubLib - https://edk2.groups.io/g/devel/message/105909. > > +/* > > + * 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 (); > > } > > } >