* [PATCH V2 0/6] Support 5-level paging in DXE long mode
@ 2019-07-24 10:00 Ni, Ray
2019-07-24 10:00 ` [PATCH V2 1/6] UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled Ni, Ray
` (7 more replies)
0 siblings, 8 replies; 21+ messages in thread
From: Ni, Ray @ 2019-07-24 10:00 UTC (permalink / raw)
To: devel
v2:
Refined the patch according to reviewers' all comments except:
0A0h cannot be changed to A0h or build fails.
A big change in this patch is Cpuid.h is moved from UefiCpuPkg to MdePkg.
The move is based on real requirement when certain modules that cannot
depend on UefiCpuPkg but needs to reference structures defined in SDM.
Ray Ni (6):
UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled
UefiCpuPkg/CpuDxe: Remove unnecessary macros
UefiCpuPkg/CpuDxe: Support parsing 5-level page table
MdeModulePkg/DxeIpl: Introduce PCD PcdUse5LevelPageTable
MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
MdeModulePkg/DxeIpl: Create 5-level page table for long mode
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 +
.../Core/DxeIplPeim/X64/VirtualMemory.c | 229 ++++++++++++------
MdeModulePkg/MdeModulePkg.dec | 7 +
MdeModulePkg/MdeModulePkg.uni | 8 +
.../Include/Register/Cpuid.h | 0
UefiCpuPkg/CpuDxe/CpuPageTable.c | 59 +++--
UefiCpuPkg/CpuDxe/CpuPageTable.h | 3 +-
UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 6 +-
UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 3 +-
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 14 +-
11 files changed, 243 insertions(+), 100 deletions(-)
rename {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h (100%)
--
2.21.0.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V2 1/6] UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled
2019-07-24 10:00 [PATCH V2 0/6] Support 5-level paging in DXE long mode Ni, Ray
@ 2019-07-24 10:00 ` Ni, Ray
2019-07-24 10:00 ` [PATCH V2 2/6] UefiCpuPkg/CpuDxe: Remove unnecessary macros Ni, Ray
` (6 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Ni, Ray @ 2019-07-24 10:00 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Laszlo Ersek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
MpInitLib is the library that's responsible to wake up APs to provide
MP PPI and Protocol services.
The patch synchronizes BSP's CR4.LA57 to each AP's CR4.LA57.
Without this change, AP may enter to GP fault when BSP's 5-level page
table is set to AP during AP wakes up.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +++++++++++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 6 +++++-
UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 3 ++-
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 14 +++++++++++++-
4 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 6f51bc4ebf..27a026632f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -790,6 +790,7 @@ FillExchangeInfoData (
volatile MP_CPU_EXCHANGE_INFO *ExchangeInfo;
UINTN Size;
IA32_SEGMENT_DESCRIPTOR *Selector;
+ IA32_CR4 Cr4;
ExchangeInfo = CpuMpData->MpCpuExchangeInfo;
ExchangeInfo->Lock = 0;
@@ -814,6 +815,18 @@ FillExchangeInfoData (
ExchangeInfo->InitializeFloatingPointUnitsAddress = (UINTN)InitializeFloatingPointUnits;
+ //
+ // We can check either CPUID(7).ECX[bit16] or check CR4.LA57[bit12]
+ // to determin whether 5-Level Paging is enabled.
+ // CPUID(7).ECX[bit16] shows CPU's capability, CR4.LA57[bit12] shows
+ // current system setting.
+ // Using latter way is simpler because it also eliminates the needs to
+ // check whether platform wants to enable it.
+ //
+ Cr4.UintN = AsmReadCr4 ();
+ ExchangeInfo->Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
+ DEBUG ((DEBUG_INFO, "%a: 5-Level Paging = %d\n", gEfiCallerBaseName, ExchangeInfo->Enable5LevelPaging));
+
//
// Get the BSP's data of GDT and IDT
//
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index f89037c59e..58859d5e20 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -1,7 +1,7 @@
/** @file
Common header file for MP Initialize Library.
- Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -185,6 +185,10 @@ typedef struct {
UINT16 ModeTransitionSegment;
UINT32 ModeHighMemory;
UINT16 ModeHighSegment;
+ //
+ // Enable5LevelPaging indicates whether 5-level paging is enabled in long mode.
+ //
+ BOOLEAN Enable5LevelPaging;
} MP_CPU_EXCHANGE_INFO;
#pragma pack()
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
index 467f54a860..58ef369342 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
@@ -1,5 +1,5 @@
;------------------------------------------------------------------------------ ;
-; Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent
;
; Module Name:
@@ -40,3 +40,4 @@ ModeTransitionMemoryLocation equ LockLocation + 94h
ModeTransitionSegmentLocation equ LockLocation + 98h
ModeHighMemoryLocation equ LockLocation + 9Ah
ModeHighSegmentLocation equ LockLocation + 9Eh
+Enable5LevelPagingLocation equ LockLocation + 0A0h
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index cea90f3d4d..87f2523e85 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -1,5 +1,5 @@
;------------------------------------------------------------------------------ ;
-; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent
;
; Module Name:
@@ -100,6 +100,18 @@ SkipEnableExecuteDisableBit:
;
mov eax, cr4
bts eax, 5
+
+ mov esi, Enable5LevelPagingLocation
+ cmp byte [ebx + esi], 0
+ jz SkipEnable5LevelPaging
+
+ ;
+ ; Enable 5 Level Paging
+ ;
+ bts eax, 12 ; Set LA57=1.
+
+SkipEnable5LevelPaging:
+
mov cr4, eax
;
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH V2 2/6] UefiCpuPkg/CpuDxe: Remove unnecessary macros
2019-07-24 10:00 [PATCH V2 0/6] Support 5-level paging in DXE long mode Ni, Ray
2019-07-24 10:00 ` [PATCH V2 1/6] UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled Ni, Ray
@ 2019-07-24 10:00 ` Ni, Ray
2019-07-24 10:00 ` [PATCH V2 3/6] UefiCpuPkg/CpuDxe: Support parsing 5-level page table Ni, Ray
` (5 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Ni, Ray @ 2019-07-24 10:00 UTC (permalink / raw)
To: devel; +Cc: Jiewen Yao, Eric Dong, Laszlo Ersek
Today's code defines macros like CR0_PG, CR0_WP, CR4_PSE, CR4_PAE
when checking whether individual bits are set in CR0 or CR4 register.
The patch changes the code to use IA32_CR0 and IA32_CR4 structure
defined in MdePkg/Include/Library/BaseLib.h so that the module
local macros can be removed.
There is no functionality impact to this change.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/CpuDxe/CpuPageTable.c | 43 ++++++++++++++++++--------------
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index c369b44f12..16a2528b55 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -1,7 +1,7 @@
/** @file
Page table management support.
- Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -21,14 +21,6 @@
#include "CpuDxe.h"
#include "CpuPageTable.h"
-///
-/// Paging registers
-///
-#define CR0_WP BIT16
-#define CR0_PG BIT31
-#define CR4_PSE BIT4
-#define CR4_PAE BIT5
-
///
/// Page Table Entry
///
@@ -161,6 +153,8 @@ GetCurrentPagingContext (
UINT32 RegEax;
CPUID_EXTENDED_CPU_SIG_EDX RegEdx;
MSR_IA32_EFER_REGISTER MsrEfer;
+ IA32_CR4 Cr4;
+ IA32_CR0 Cr0;
//
// Don't retrieve current paging context from processor if in SMM mode.
@@ -172,21 +166,24 @@ GetCurrentPagingContext (
} else {
mPagingContext.MachineType = IMAGE_FILE_MACHINE_I386;
}
- if ((AsmReadCr0 () & CR0_PG) != 0) {
+
+ Cr0.UintN = AsmReadCr0 ();
+ Cr4.UintN = AsmReadCr4 ();
+
+ if (Cr0.Bits.PG != 0) {
mPagingContext.ContextData.X64.PageTableBase = (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64);
} else {
mPagingContext.ContextData.X64.PageTableBase = 0;
}
-
- if ((AsmReadCr4 () & CR4_PSE) != 0) {
+ if (Cr0.Bits.WP != 0) {
+ mPagingContext.ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_WP_ENABLE;
+ }
+ if (Cr4.Bits.PSE != 0) {
mPagingContext.ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PSE;
}
- if ((AsmReadCr4 () & CR4_PAE) != 0) {
+ if (Cr4.Bits.PAE != 0) {
mPagingContext.ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE;
}
- if ((AsmReadCr0 () & CR0_WP) != 0) {
- mPagingContext.ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_WP_ENABLE;
- }
AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
@@ -581,12 +578,14 @@ IsReadOnlyPageWriteProtected (
VOID
)
{
+ IA32_CR0 Cr0;
//
// To avoid unforseen consequences, don't touch paging settings in SMM mode
// in this driver.
//
if (!IsInSmm ()) {
- return ((AsmReadCr0 () & CR0_WP) != 0);
+ Cr0.UintN = AsmReadCr0 ();
+ return (BOOLEAN) (Cr0.Bits.WP != 0);
}
return FALSE;
}
@@ -599,12 +598,15 @@ DisableReadOnlyPageWriteProtect (
VOID
)
{
+ IA32_CR0 Cr0;
//
// To avoid unforseen consequences, don't touch paging settings in SMM mode
// in this driver.
//
if (!IsInSmm ()) {
- AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
+ Cr0.UintN = AsmReadCr0 ();
+ Cr0.Bits.WP = 0;
+ AsmWriteCr0 (Cr0.UintN);
}
}
@@ -616,12 +618,15 @@ EnableReadOnlyPageWriteProtect (
VOID
)
{
+ IA32_CR0 Cr0;
//
// To avoid unforseen consequences, don't touch paging settings in SMM mode
// in this driver.
//
if (!IsInSmm ()) {
- AsmWriteCr0 (AsmReadCr0 () | CR0_WP);
+ Cr0.UintN = AsmReadCr0 ();
+ Cr0.Bits.WP = 1;
+ AsmWriteCr0 (Cr0.UintN);
}
}
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH V2 3/6] UefiCpuPkg/CpuDxe: Support parsing 5-level page table
2019-07-24 10:00 [PATCH V2 0/6] Support 5-level paging in DXE long mode Ni, Ray
2019-07-24 10:00 ` [PATCH V2 1/6] UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled Ni, Ray
2019-07-24 10:00 ` [PATCH V2 2/6] UefiCpuPkg/CpuDxe: Remove unnecessary macros Ni, Ray
@ 2019-07-24 10:00 ` Ni, Ray
2019-07-24 10:00 ` [PATCH V2 4/6] MdeModulePkg/DxeIpl: Introduce PCD PcdUse5LevelPageTable Ni, Ray
` (4 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Ni, Ray @ 2019-07-24 10:00 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Laszlo Ersek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/CpuDxe/CpuPageTable.c | 18 +++++++++++++++++-
UefiCpuPkg/CpuDxe/CpuPageTable.h | 3 ++-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index 16a2528b55..36ce90d66c 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -184,6 +184,9 @@ GetCurrentPagingContext (
if (Cr4.Bits.PAE != 0) {
mPagingContext.ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE;
}
+ if (Cr4.Bits.LA57 != 0) {
+ mPagingContext.ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_5_LEVEL;
+ }
AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL);
if (RegEax >= CPUID_EXTENDED_CPU_SIG) {
@@ -273,14 +276,17 @@ GetPageTableEntry (
UINTN Index2;
UINTN Index3;
UINTN Index4;
+ UINTN Index5;
UINT64 *L1PageTable;
UINT64 *L2PageTable;
UINT64 *L3PageTable;
UINT64 *L4PageTable;
+ UINT64 *L5PageTable;
UINT64 AddressEncMask;
ASSERT (PagingContext != NULL);
+ Index5 = ((UINTN)RShiftU64 (Address, 48)) & PAGING_PAE_INDEX_MASK;
Index4 = ((UINTN)RShiftU64 (Address, 39)) & PAGING_PAE_INDEX_MASK;
Index3 = ((UINTN)Address >> 30) & PAGING_PAE_INDEX_MASK;
Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK;
@@ -291,7 +297,17 @@ GetPageTableEntry (
AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask) & PAGING_1G_ADDRESS_MASK_64;
if (PagingContext->MachineType == IMAGE_FILE_MACHINE_X64) {
- L4PageTable = (UINT64 *)(UINTN)PagingContext->ContextData.X64.PageTableBase;
+ if ((PagingContext->ContextData.X64.Attributes & PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_5_LEVEL) != 0) {
+ L5PageTable = (UINT64 *)(UINTN)PagingContext->ContextData.X64.PageTableBase;
+ if (L5PageTable[Index5] == 0) {
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+
+ L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] & ~AddressEncMask & PAGING_4K_ADDRESS_MASK_64);
+ } else {
+ L4PageTable = (UINT64 *)(UINTN)PagingContext->ContextData.X64.PageTableBase;
+ }
if (L4PageTable[Index4] == 0) {
*PageAttribute = PageNone;
return NULL;
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.h b/UefiCpuPkg/CpuDxe/CpuPageTable.h
index 02d62f2b14..f845956f73 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.h
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.h
@@ -1,7 +1,7 @@
/** @file
Page table management header file.
- Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -14,6 +14,7 @@
#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PSE BIT0
#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE BIT1
#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAGE_1G_SUPPORT BIT2
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_5_LEVEL BIT3
#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_WP_ENABLE BIT30
#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED BIT31
// Other bits are reserved for future use
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH V2 4/6] MdeModulePkg/DxeIpl: Introduce PCD PcdUse5LevelPageTable
2019-07-24 10:00 [PATCH V2 0/6] Support 5-level paging in DXE long mode Ni, Ray
` (2 preceding siblings ...)
2019-07-24 10:00 ` [PATCH V2 3/6] UefiCpuPkg/CpuDxe: Support parsing 5-level page table Ni, Ray
@ 2019-07-24 10:00 ` Ni, Ray
2019-07-24 10:00 ` [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg Ni, Ray
` (3 subsequent siblings)
7 siblings, 0 replies; 21+ messages in thread
From: Ni, Ray @ 2019-07-24 10:00 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Laszlo Ersek, Hao A Wu, Jian J Wang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
The PCD indicates if 5-Level Paging will be enabled in long mode.
5-Level Paging will not be enabled when the PCD is TRUE but CPU
doesn't support 5-Level Paging.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
---
MdeModulePkg/MdeModulePkg.dec | 7 +++++++
MdeModulePkg/MdeModulePkg.uni | 8 ++++++++
2 files changed, 15 insertions(+)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 12e0bbf579..21388595a9 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1991,6 +1991,13 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
# @Prompt The address mask when memory encryption is enabled.
gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0|UINT64|0x30001047
+ ## Indicates if 5-Level Paging will be enabled in long mode. 5-Level Paging will not be enabled
+ # when the PCD is TRUE but CPU doesn't support 5-Level Paging.
+ # TRUE - 5-Level Paging will be enabled.<BR>
+ # FALSE - 5-Level Paging will not be enabled.<BR>
+ # @Prompt Enable 5-Level Paging support in long mode.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable|FALSE|BOOLEAN|0x0001105F
+
## Capsule In Ram is to use memory to deliver the capsules that will be processed after system
# reset.<BR><BR>
# This PCD indicates if the Capsule In Ram is supported.<BR>
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 2bd1ad29f2..3a1c88e4c5 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -1298,3 +1298,11 @@
"It is used to set VPD region base address. So, it can't be DynamicExVpd PCD. Its value is"
"required to be accessed in PcdDxe driver entry point. So, its value must be set in PEI phase."
"It can't depend on EFI variable service, and can't be DynamicExHii PCD."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUse5LevelPageTable_PROMPT #language en-US "Enable 5-Level Paging support in long mode"
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdUse5LevelPageTable_HELP #language en-US "Indicates if 5-Level Paging will be enabled in long mode. 5-Level Paging will not be enabled"
+ "when the PCD is TRUE but CPU doesn't support 5-Level Paging."
+ " TRUE - 5-Level Paging will be enabled."
+ " FALSE - 5-Level Paging will not be enabled."
+
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
2019-07-24 10:00 [PATCH V2 0/6] Support 5-level paging in DXE long mode Ni, Ray
` (3 preceding siblings ...)
2019-07-24 10:00 ` [PATCH V2 4/6] MdeModulePkg/DxeIpl: Introduce PCD PcdUse5LevelPageTable Ni, Ray
@ 2019-07-24 10:00 ` Ni, Ray
2019-07-30 3:29 ` Liming Gao
2019-07-24 10:00 ` [PATCH V2 6/6] MdeModulePkg/DxeIpl: Create 5-level page table for long mode Ni, Ray
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Ni, Ray @ 2019-07-24 10:00 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Eric Dong, Laszlo Ersek
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
MdeModulePkg/DxeIpl needs to get CPUID output for CPU
5-level paging capability detection.
In order to use the macros/structures defined in
UefiCpuPkg/Include/Register/Cpuid.h, the patch moves
Cpuid.h to MdePkg/Include/Register/ directory.
Because almost every module depends on MdePkg.dec in its
INF file, the move almost has no impact.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
{UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h (100%)
diff --git a/UefiCpuPkg/Include/Register/Cpuid.h b/MdePkg/Include/Register/Cpuid.h
similarity index 100%
rename from UefiCpuPkg/Include/Register/Cpuid.h
rename to MdePkg/Include/Register/Cpuid.h
--
2.21.0.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH V2 6/6] MdeModulePkg/DxeIpl: Create 5-level page table for long mode
2019-07-24 10:00 [PATCH V2 0/6] Support 5-level paging in DXE long mode Ni, Ray
` (4 preceding siblings ...)
2019-07-24 10:00 ` [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg Ni, Ray
@ 2019-07-24 10:00 ` Ni, Ray
2019-07-24 17:23 ` [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE " Michael D Kinney
2019-07-26 10:14 ` Laszlo Ersek
7 siblings, 0 replies; 21+ messages in thread
From: Ni, Ray @ 2019-07-24 10:00 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Laszlo Ersek, Hao A Wu, Jian J Wang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
DxeIpl is responsible to create page table for DXE phase running
either in long mode or in 32bit mode with certain protection
mechanism enabled (refer to ToBuildPageTable()).
The patch updates DxeIpl to create 5-level page table for DXE phase
running in long mode when PcdUse5LevelPageTable is TRUE and CPU
supports 5-level page table.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
---
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 +
.../Core/DxeIplPeim/X64/VirtualMemory.c | 229 ++++++++++++------
2 files changed, 153 insertions(+), 77 deletions(-)
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index abc3217b01..98bc17fc9d 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -110,6 +110,7 @@ [Pcd.IA32,Pcd.X64]
gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdUse5LevelPageTable ## SOMETIMES_CONSUMES
[Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index edc38e4525..e049c74a2e 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -15,13 +15,14 @@
2) IA-32 Intel(R) Architecture Software Developer's Manual Volume 2:Instruction Set Reference, Intel
3) IA-32 Intel(R) Architecture Software Developer's Manual Volume 3:System Programmer's Guide, Intel
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
+#include <Register/Cpuid.h>
#include "DxeIpl.h"
#include "VirtualMemory.h"
@@ -626,14 +627,18 @@ CreateIdentityMappingPageTables (
)
{
UINT32 RegEax;
+ CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_ECX EcxFlags;
UINT32 RegEdx;
UINT8 PhysicalAddressBits;
EFI_PHYSICAL_ADDRESS PageAddress;
+ UINTN IndexOfPml5Entries;
UINTN IndexOfPml4Entries;
UINTN IndexOfPdpEntries;
UINTN IndexOfPageDirectoryEntries;
+ UINT32 NumberOfPml5EntriesNeeded;
UINT32 NumberOfPml4EntriesNeeded;
UINT32 NumberOfPdpEntriesNeeded;
+ PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel5Entry;
PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry;
PAGE_MAP_AND_DIRECTORY_POINTER *PageMap;
PAGE_MAP_AND_DIRECTORY_POINTER *PageDirectoryPointerEntry;
@@ -641,9 +646,11 @@ CreateIdentityMappingPageTables (
UINTN TotalPagesNum;
UINTN BigPageAddress;
VOID *Hob;
+ BOOLEAN Page5LevelSupport;
BOOLEAN Page1GSupport;
PAGE_TABLE_1G_ENTRY *PageDirectory1GEntry;
UINT64 AddressEncMask;
+ IA32_CR4 Cr4;
//
// Make sure AddressEncMask is contained to smallest supported address field
@@ -677,33 +684,68 @@ CreateIdentityMappingPageTables (
}
}
+ Page5LevelSupport = FALSE;
+ if (PcdGetBool (PcdUse5LevelPageTable)) {
+ AsmCpuidEx (
+ CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS, CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS_SUB_LEAF_INFO, NULL,
+ &EcxFlags.Uint32, NULL, NULL
+ );
+ if (EcxFlags.Bits.FiveLevelPage != 0) {
+ Page5LevelSupport = TRUE;
+ }
+ }
+
+ DEBUG ((DEBUG_INFO, "AddressBits=%u 5LevelPaging=%u 1GPage=%u\n", PhysicalAddressBits, Page5LevelSupport, Page1GSupport));
+
//
- // IA-32e paging translates 48-bit linear addresses to 52-bit physical addresses.
+ // IA-32e paging translates 48-bit linear addresses to 52-bit physical addresses
+ // when 5-Level Paging is disabled,
+ // due to either unsupported by HW, or disabled by PCD.
//
ASSERT (PhysicalAddressBits <= 52);
- if (PhysicalAddressBits > 48) {
+ if (!Page5LevelSupport && PhysicalAddressBits > 48) {
PhysicalAddressBits = 48;
}
//
// Calculate the table entries needed.
//
- if (PhysicalAddressBits <= 39 ) {
- NumberOfPml4EntriesNeeded = 1;
- NumberOfPdpEntriesNeeded = (UINT32)LShiftU64 (1, (PhysicalAddressBits - 30));
- } else {
- NumberOfPml4EntriesNeeded = (UINT32)LShiftU64 (1, (PhysicalAddressBits - 39));
- NumberOfPdpEntriesNeeded = 512;
+ NumberOfPml5EntriesNeeded = 1;
+ if (PhysicalAddressBits > 48) {
+ NumberOfPml5EntriesNeeded = (UINT32) LShiftU64 (1, PhysicalAddressBits - 48);
+ PhysicalAddressBits = 48;
+ }
+
+ NumberOfPml4EntriesNeeded = 1;
+ if (PhysicalAddressBits > 39) {
+ NumberOfPml4EntriesNeeded = (UINT32) LShiftU64 (1, PhysicalAddressBits - 39);
+ PhysicalAddressBits = 39;
}
+ NumberOfPdpEntriesNeeded = 1;
+ ASSERT (PhysicalAddressBits > 30);
+ NumberOfPdpEntriesNeeded = (UINT32) LShiftU64 (1, PhysicalAddressBits - 30);
+
//
// Pre-allocate big pages to avoid later allocations.
//
if (!Page1GSupport) {
- TotalPagesNum = (NumberOfPdpEntriesNeeded + 1) * NumberOfPml4EntriesNeeded + 1;
+ TotalPagesNum = ((NumberOfPdpEntriesNeeded + 1) * NumberOfPml4EntriesNeeded + 1) * NumberOfPml5EntriesNeeded + 1;
} else {
- TotalPagesNum = NumberOfPml4EntriesNeeded + 1;
+ TotalPagesNum = (NumberOfPml4EntriesNeeded + 1) * NumberOfPml5EntriesNeeded + 1;
+ }
+
+ //
+ // Substract the one page occupied by PML5 entries if 5-Level Paging is disabled.
+ //
+ if (!Page5LevelSupport) {
+ TotalPagesNum--;
}
+
+ DEBUG ((DEBUG_INFO, "Pml5=%u Pml4=%u Pdp=%u TotalPage=%u\n",
+ NumberOfPml5EntriesNeeded, NumberOfPml4EntriesNeeded,
+ NumberOfPdpEntriesNeeded, TotalPagesNum));
+
BigPageAddress = (UINTN) AllocatePageTableMemory (TotalPagesNum);
ASSERT (BigPageAddress != 0);
@@ -711,92 +753,125 @@ CreateIdentityMappingPageTables (
// By architecture only one PageMapLevel4 exists - so lets allocate storage for it.
//
PageMap = (VOID *) BigPageAddress;
- BigPageAddress += SIZE_4KB;
-
- PageMapLevel4Entry = PageMap;
- PageAddress = 0;
- for (IndexOfPml4Entries = 0; IndexOfPml4Entries < NumberOfPml4EntriesNeeded; IndexOfPml4Entries++, PageMapLevel4Entry++) {
+ if (Page5LevelSupport) {
//
- // Each PML4 entry points to a page of Page Directory Pointer entires.
- // So lets allocate space for them and fill them in in the IndexOfPdpEntries loop.
+ // By architecture only one PageMapLevel5 exists - so lets allocate storage for it.
//
- PageDirectoryPointerEntry = (VOID *) BigPageAddress;
- BigPageAddress += SIZE_4KB;
+ PageMapLevel5Entry = PageMap;
+ BigPageAddress += SIZE_4KB;
+ }
+ PageAddress = 0;
+ for ( IndexOfPml5Entries = 0
+ ; IndexOfPml5Entries < NumberOfPml5EntriesNeeded
+ ; IndexOfPml5Entries++, PageMapLevel5Entry++) {
//
- // Make a PML4 Entry
+ // Each PML5 entry points to a page of PML4 entires.
+ // So lets allocate space for them and fill them in in the IndexOfPml4Entries loop.
+ // When 5-Level Paging is disabled, below allocation happens only once.
//
- PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)PageDirectoryPointerEntry | AddressEncMask;
- PageMapLevel4Entry->Bits.ReadWrite = 1;
- PageMapLevel4Entry->Bits.Present = 1;
+ PageMapLevel4Entry = (VOID *) BigPageAddress;
+ BigPageAddress += SIZE_4KB;
- if (Page1GSupport) {
- PageDirectory1GEntry = (VOID *) PageDirectoryPointerEntry;
+ if (Page5LevelSupport) {
+ //
+ // Make a PML5 Entry
+ //
+ PageMapLevel5Entry->Uint64 = (UINT64) (UINTN) PageMapLevel4Entry | AddressEncMask;
+ PageMapLevel5Entry->Bits.ReadWrite = 1;
+ PageMapLevel5Entry->Bits.Present = 1;
+ }
- for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectory1GEntry++, PageAddress += SIZE_1GB) {
- if (ToSplitPageTable (PageAddress, SIZE_1GB, StackBase, StackSize)) {
- Split1GPageTo2M (PageAddress, (UINT64 *) PageDirectory1GEntry, StackBase, StackSize);
- } else {
- //
- // Fill in the Page Directory entries
- //
- PageDirectory1GEntry->Uint64 = (UINT64)PageAddress | AddressEncMask;
- PageDirectory1GEntry->Bits.ReadWrite = 1;
- PageDirectory1GEntry->Bits.Present = 1;
- PageDirectory1GEntry->Bits.MustBe1 = 1;
- }
- }
- } else {
- for (IndexOfPdpEntries = 0; IndexOfPdpEntries < NumberOfPdpEntriesNeeded; IndexOfPdpEntries++, PageDirectoryPointerEntry++) {
- //
- // Each Directory Pointer entries points to a page of Page Directory entires.
- // So allocate space for them and fill them in in the IndexOfPageDirectoryEntries loop.
- //
- PageDirectoryEntry = (VOID *) BigPageAddress;
- BigPageAddress += SIZE_4KB;
+ for ( IndexOfPml4Entries = 0
+ ; IndexOfPml4Entries < (NumberOfPml5EntriesNeeded == 1 ? NumberOfPml4EntriesNeeded : 512)
+ ; IndexOfPml4Entries++, PageMapLevel4Entry++) {
+ //
+ // Each PML4 entry points to a page of Page Directory Pointer entires.
+ // So lets allocate space for them and fill them in in the IndexOfPdpEntries loop.
+ //
+ PageDirectoryPointerEntry = (VOID *) BigPageAddress;
+ BigPageAddress += SIZE_4KB;
- //
- // Fill in a Page Directory Pointer Entries
- //
- PageDirectoryPointerEntry->Uint64 = (UINT64)(UINTN)PageDirectoryEntry | AddressEncMask;
- PageDirectoryPointerEntry->Bits.ReadWrite = 1;
- PageDirectoryPointerEntry->Bits.Present = 1;
+ //
+ // Make a PML4 Entry
+ //
+ PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)PageDirectoryPointerEntry | AddressEncMask;
+ PageMapLevel4Entry->Bits.ReadWrite = 1;
+ PageMapLevel4Entry->Bits.Present = 1;
- for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PageAddress += SIZE_2MB) {
- if (ToSplitPageTable (PageAddress, SIZE_2MB, StackBase, StackSize)) {
- //
- // Need to split this 2M page that covers NULL or stack range.
- //
- Split2MPageTo4K (PageAddress, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
+ if (Page1GSupport) {
+ 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);
} else {
//
// Fill in the Page Directory entries
//
- PageDirectoryEntry->Uint64 = (UINT64)PageAddress | AddressEncMask;
- PageDirectoryEntry->Bits.ReadWrite = 1;
- PageDirectoryEntry->Bits.Present = 1;
- PageDirectoryEntry->Bits.MustBe1 = 1;
+ PageDirectory1GEntry->Uint64 = (UINT64)PageAddress | AddressEncMask;
+ PageDirectory1GEntry->Bits.ReadWrite = 1;
+ PageDirectory1GEntry->Bits.Present = 1;
+ PageDirectory1GEntry->Bits.MustBe1 = 1;
}
}
- }
+ } else {
+ for ( IndexOfPdpEntries = 0
+ ; IndexOfPdpEntries < (NumberOfPml4EntriesNeeded == 1 ? NumberOfPdpEntriesNeeded : 512)
+ ; IndexOfPdpEntries++, PageDirectoryPointerEntry++) {
+ //
+ // Each Directory Pointer entries points to a page of Page Directory entires.
+ // So allocate space for them and fill them in in the IndexOfPageDirectoryEntries loop.
+ //
+ PageDirectoryEntry = (VOID *) BigPageAddress;
+ BigPageAddress += SIZE_4KB;
- for (; IndexOfPdpEntries < 512; IndexOfPdpEntries++, PageDirectoryPointerEntry++) {
- ZeroMem (
- PageDirectoryPointerEntry,
- sizeof(PAGE_MAP_AND_DIRECTORY_POINTER)
- );
+ //
+ // Fill in a Page Directory Pointer Entries
+ //
+ PageDirectoryPointerEntry->Uint64 = (UINT64)(UINTN)PageDirectoryEntry | AddressEncMask;
+ PageDirectoryPointerEntry->Bits.ReadWrite = 1;
+ PageDirectoryPointerEntry->Bits.Present = 1;
+
+ for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PageAddress += SIZE_2MB) {
+ if (ToSplitPageTable (PageAddress, SIZE_2MB, StackBase, StackSize)) {
+ //
+ // Need to split this 2M page that covers NULL or stack range.
+ //
+ Split2MPageTo4K (PageAddress, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
+ } else {
+ //
+ // Fill in the Page Directory entries
+ //
+ PageDirectoryEntry->Uint64 = (UINT64)PageAddress | AddressEncMask;
+ PageDirectoryEntry->Bits.ReadWrite = 1;
+ PageDirectoryEntry->Bits.Present = 1;
+ PageDirectoryEntry->Bits.MustBe1 = 1;
+ }
+ }
+ }
+
+ //
+ // Fill with null entry for unused PDPTE
+ //
+ ZeroMem (PageDirectoryPointerEntry, (512 - IndexOfPdpEntries) * sizeof(PAGE_MAP_AND_DIRECTORY_POINTER));
}
}
+
+ //
+ // For the PML4 entries we are not using fill in a null entry.
+ //
+ ZeroMem (PageMapLevel4Entry, (512 - IndexOfPml4Entries) * sizeof (PAGE_MAP_AND_DIRECTORY_POINTER));
}
- //
- // For the PML4 entries we are not using fill in a null entry.
- //
- for (; IndexOfPml4Entries < 512; IndexOfPml4Entries++, PageMapLevel4Entry++) {
- ZeroMem (
- PageMapLevel4Entry,
- sizeof (PAGE_MAP_AND_DIRECTORY_POINTER)
- );
+ if (Page5LevelSupport) {
+ Cr4.UintN = AsmReadCr4 ();
+ Cr4.Bits.LA57 = 1;
+ AsmWriteCr4 (Cr4.UintN);
+ //
+ // For the PML5 entries we are not using fill in a null entry.
+ //
+ ZeroMem (PageMapLevel5Entry, (512 - IndexOfPml5Entries) * sizeof (PAGE_MAP_AND_DIRECTORY_POINTER));
}
//
--
2.21.0.windows.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode
2019-07-24 10:00 [PATCH V2 0/6] Support 5-level paging in DXE long mode Ni, Ray
` (5 preceding siblings ...)
2019-07-24 10:00 ` [PATCH V2 6/6] MdeModulePkg/DxeIpl: Create 5-level page table for long mode Ni, Ray
@ 2019-07-24 17:23 ` Michael D Kinney
2019-07-25 0:45 ` Ni, Ray
2019-07-26 10:14 ` Laszlo Ersek
7 siblings, 1 reply; 21+ messages in thread
From: Michael D Kinney @ 2019-07-24 17:23 UTC (permalink / raw)
To: devel@edk2.groups.io, Ni, Ray, Kinney, Michael D
Hi Ray,
Given that there may be register definitions for other
CPUs or devices added to MdePkg in the future, should
an extra directory level be added? Doing that would
break source compatibility for existing components that
use #include <Register/Cpuid.h> from UefiCpuPkg. We could
keep Cpuid.h in UefiCpuPkg, and it could be a #include
of the new Cpuid.h file in the MdePkg in the extended path.
Also, should CpuId.h be included from BaseLib.h inside:
#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
This would make all CPUID related register definitions
available to components the needs BaseLib to call
AsmCpuId() or AsmCpuIdEx()?
We could also move the CRx, FLAGS/EFLAGS/descriptor
structures out of BaseLib.h into include files that
are peers to Cpuid.h and could be updated based on
public spec updates.
Thanks,
Mike
> -----Original Message-----
> From: devel@edk2.groups.io
> [mailto:devel@edk2.groups.io] On Behalf Of Ni, Ray
> Sent: Wednesday, July 24, 2019 3:00 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH V2 0/6] Support 5-level
> paging in DXE long mode
>
> v2:
> Refined the patch according to reviewers' all
> comments except:
> 0A0h cannot be changed to A0h or build fails.
> A big change in this patch is Cpuid.h is moved from
> UefiCpuPkg to MdePkg.
> The move is based on real requirement when certain
> modules that cannot
> depend on UefiCpuPkg but needs to reference
> structures defined in SDM.
>
> Ray Ni (6):
> UefiCpuPkg/MpInitLib: Enable 5-level paging for AP
> when BSP's enabled
> UefiCpuPkg/CpuDxe: Remove unnecessary macros
> UefiCpuPkg/CpuDxe: Support parsing 5-level page table
> MdeModulePkg/DxeIpl: Introduce PCD
> PcdUse5LevelPageTable
> MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to
> MdePkg
> MdeModulePkg/DxeIpl: Create 5-level page table for
> long mode
>
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 +
> .../Core/DxeIplPeim/X64/VirtualMemory.c | 229
> ++++++++++++------
> MdeModulePkg/MdeModulePkg.dec | 7 +
> MdeModulePkg/MdeModulePkg.uni | 8 +
> .../Include/Register/Cpuid.h | 0
> UefiCpuPkg/CpuDxe/CpuPageTable.c | 59
> +++--
> UefiCpuPkg/CpuDxe/CpuPageTable.h | 3 +-
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 6 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 3 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 14 +-
> 11 files changed, 243 insertions(+), 100 deletions(-)
> rename {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h
> (100%)
>
> --
> 2.21.0.windows.1
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode
2019-07-24 17:23 ` [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE " Michael D Kinney
@ 2019-07-25 0:45 ` Ni, Ray
2019-07-25 0:52 ` Michael D Kinney
0 siblings, 1 reply; 21+ messages in thread
From: Ni, Ray @ 2019-07-25 0:45 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
Mike,
Are you suggesting that
1. Copy Cpuid.h in MdePkg/Include/IndustryStandard/
2. Change UefiCpuPkg/Include/Register/Cpuid.h to just include <IndustryStandard/Cpuid.h>
It looks like a potential issue that there are two "Cpuid.h" public header file in different folders.
But given the include pattern used: "<Register/Cpuid.h>" VS "<IndustryStandard/Cpuid.h>", the risk
people may include wrong file or compilers don't know which file to use is zero.
Is that what you think?
Thanks,
Ray
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Thursday, July 25, 2019 1:23 AM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode
>
> Hi Ray,
>
> Given that there may be register definitions for other
> CPUs or devices added to MdePkg in the future, should
> an extra directory level be added? Doing that would
> break source compatibility for existing components that
> use #include <Register/Cpuid.h> from UefiCpuPkg. We could
> keep Cpuid.h in UefiCpuPkg, and it could be a #include
> of the new Cpuid.h file in the MdePkg in the extended path.
>
> Also, should CpuId.h be included from BaseLib.h inside:
>
> #if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
>
> This would make all CPUID related register definitions
> available to components the needs BaseLib to call
> AsmCpuId() or AsmCpuIdEx()?
>
> We could also move the CRx, FLAGS/EFLAGS/descriptor
> structures out of BaseLib.h into include files that
> are peers to Cpuid.h and could be updated based on
> public spec updates.
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io
> > [mailto:devel@edk2.groups.io] On Behalf Of Ni, Ray
> > Sent: Wednesday, July 24, 2019 3:00 AM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH V2 0/6] Support 5-level
> > paging in DXE long mode
> >
> > v2:
> > Refined the patch according to reviewers' all
> > comments except:
> > 0A0h cannot be changed to A0h or build fails.
> > A big change in this patch is Cpuid.h is moved from
> > UefiCpuPkg to MdePkg.
> > The move is based on real requirement when certain
> > modules that cannot
> > depend on UefiCpuPkg but needs to reference
> > structures defined in SDM.
> >
> > Ray Ni (6):
> > UefiCpuPkg/MpInitLib: Enable 5-level paging for AP
> > when BSP's enabled
> > UefiCpuPkg/CpuDxe: Remove unnecessary macros
> > UefiCpuPkg/CpuDxe: Support parsing 5-level page table
> > MdeModulePkg/DxeIpl: Introduce PCD
> > PcdUse5LevelPageTable
> > MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to
> > MdePkg
> > MdeModulePkg/DxeIpl: Create 5-level page table for
> > long mode
> >
> > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 +
> > .../Core/DxeIplPeim/X64/VirtualMemory.c | 229
> > ++++++++++++------
> > MdeModulePkg/MdeModulePkg.dec | 7 +
> > MdeModulePkg/MdeModulePkg.uni | 8 +
> > .../Include/Register/Cpuid.h | 0
> > UefiCpuPkg/CpuDxe/CpuPageTable.c | 59
> > +++--
> > UefiCpuPkg/CpuDxe/CpuPageTable.h | 3 +-
> > UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +
> > UefiCpuPkg/Library/MpInitLib/MpLib.h | 6 +-
> > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 3 +-
> > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 14 +-
> > 11 files changed, 243 insertions(+), 100 deletions(-)
> > rename {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h
> > (100%)
> >
> > --
> > 2.21.0.windows.1
> >
> >
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode
2019-07-25 0:45 ` Ni, Ray
@ 2019-07-25 0:52 ` Michael D Kinney
2019-07-25 5:39 ` Ni, Ray
0 siblings, 1 reply; 21+ messages in thread
From: Michael D Kinney @ 2019-07-25 0:52 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io, Kinney, Michael D
Ray,
I think the use of Include/Register is good for this
type of content. But we may need a Vendor directory.
The general form would be:
Include/Register/<Vendor>/<RegisterFile>.h
Since the definitions being discussed here are from
an Intel public document, perhaps the path should be:
Include/Register/Intel/Cpuid.h
Thanks,
Mike
> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, July 24, 2019 5:46 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
> level paging in DXE long mode
>
> Mike,
> Are you suggesting that
> 1. Copy Cpuid.h in MdePkg/Include/IndustryStandard/ 2.
> Change UefiCpuPkg/Include/Register/Cpuid.h to just
> include <IndustryStandard/Cpuid.h>
>
> It looks like a potential issue that there are two
> "Cpuid.h" public header file in different folders.
> But given the include pattern used:
> "<Register/Cpuid.h>" VS "<IndustryStandard/Cpuid.h>",
> the risk people may include wrong file or compilers
> don't know which file to use is zero.
>
> Is that what you think?
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Thursday, July 25, 2019 1:23 AM
> > To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>;
> Kinney, Michael
> > D <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
> level paging in DXE
> > long mode
> >
> > Hi Ray,
> >
> > Given that there may be register definitions for
> other CPUs or devices
> > added to MdePkg in the future, should an extra
> directory level be
> > added? Doing that would break source compatibility
> for existing
> > components that use #include <Register/Cpuid.h> from
> UefiCpuPkg. We
> > could keep Cpuid.h in UefiCpuPkg, and it could be a
> #include of the
> > new Cpuid.h file in the MdePkg in the extended path.
> >
> > Also, should CpuId.h be included from BaseLib.h
> inside:
> >
> > #if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
> >
> > This would make all CPUID related register
> definitions available to
> > components the needs BaseLib to call
> > AsmCpuId() or AsmCpuIdEx()?
> >
> > We could also move the CRx, FLAGS/EFLAGS/descriptor
> structures out of
> > BaseLib.h into include files that are peers to
> Cpuid.h and could be
> > updated based on public spec updates.
> >
> > Thanks,
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io
> > > [mailto:devel@edk2.groups.io] On Behalf Of Ni, Ray
> > > Sent: Wednesday, July 24, 2019 3:00 AM
> > > To: devel@edk2.groups.io
> > > Subject: [edk2-devel] [PATCH V2 0/6] Support 5-
> level paging in DXE
> > > long mode
> > >
> > > v2:
> > > Refined the patch according to reviewers' all
> comments except:
> > > 0A0h cannot be changed to A0h or build fails.
> > > A big change in this patch is Cpuid.h is moved
> from UefiCpuPkg to
> > > MdePkg.
> > > The move is based on real requirement when
> certain modules that
> > > cannot
> > > depend on UefiCpuPkg but needs to reference
> structures defined in
> > > SDM.
> > >
> > > Ray Ni (6):
> > > UefiCpuPkg/MpInitLib: Enable 5-level paging for
> AP when BSP's
> > > enabled
> > > UefiCpuPkg/CpuDxe: Remove unnecessary macros
> > > UefiCpuPkg/CpuDxe: Support parsing 5-level page
> table
> > > MdeModulePkg/DxeIpl: Introduce PCD
> PcdUse5LevelPageTable
> > > MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to
> MdePkg
> > > MdeModulePkg/DxeIpl: Create 5-level page table
> for long mode
> > >
> > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf |
> 1 +
> > > .../Core/DxeIplPeim/X64/VirtualMemory.c |
> 229
> > > ++++++++++++------
> > > MdeModulePkg/MdeModulePkg.dec |
> 7 +
> > > MdeModulePkg/MdeModulePkg.uni |
> 8 +
> > > .../Include/Register/Cpuid.h |
> 0
> > > UefiCpuPkg/CpuDxe/CpuPageTable.c |
> 59
> > > +++--
> > > UefiCpuPkg/CpuDxe/CpuPageTable.h |
> 3 +-
> > > UefiCpuPkg/Library/MpInitLib/MpLib.c |
> 13 +
> > > UefiCpuPkg/Library/MpInitLib/MpLib.h |
> 6 +-
> > > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc |
> 3 +-
> > > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm |
> 14 +-
> > > 11 files changed, 243 insertions(+), 100
> deletions(-) rename
> > > {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h
> > > (100%)
> > >
> > > --
> > > 2.21.0.windows.1
> > >
> > >
> > >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode
2019-07-25 0:52 ` Michael D Kinney
@ 2019-07-25 5:39 ` Ni, Ray
2019-07-25 15:55 ` Michael D Kinney
0 siblings, 1 reply; 21+ messages in thread
From: Ni, Ray @ 2019-07-25 5:39 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
Mike,
All the CPUID definitions also applies to AMD processors.
There are two Cpuid.h in UefiCpuPkg today.
1. UefiCpuPkg/Include/Register/Cpuid.h
2. UefiCpuPkg/Include/Register/Amd/Cpuid.h
The second one contains additional structures needed by AMD processor.
But first one also applies to AMD processor.
Can we just put Cpuid.h under MdePkg/Include/Register/ because they are
common to both Intel and AMD?
Thanks,
Ray
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Thursday, July 25, 2019 8:52 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long
> mode
>
> Ray,
>
> I think the use of Include/Register is good for this type of content. But we
> may need a Vendor directory.
>
> The general form would be:
>
> Include/Register/<Vendor>/<RegisterFile>.h
>
> Since the definitions being discussed here are from an Intel public document,
> perhaps the path should be:
>
> Include/Register/Intel/Cpuid.h
>
> Thanks,
>
> Mike
>
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Wednesday, July 24, 2019 5:46 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5- level paging in
> > DXE long mode
> >
> > Mike,
> > Are you suggesting that
> > 1. Copy Cpuid.h in MdePkg/Include/IndustryStandard/ 2.
> > Change UefiCpuPkg/Include/Register/Cpuid.h to just include
> > <IndustryStandard/Cpuid.h>
> >
> > It looks like a potential issue that there are two "Cpuid.h" public
> > header file in different folders.
> > But given the include pattern used:
> > "<Register/Cpuid.h>" VS "<IndustryStandard/Cpuid.h>", the risk people
> > may include wrong file or compilers don't know which file to use is
> > zero.
> >
> > Is that what you think?
> >
> > Thanks,
> > Ray
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D
> > > Sent: Thursday, July 25, 2019 1:23 AM
> > > To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>;
> > Kinney, Michael
> > > D <michael.d.kinney@intel.com>
> > > Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
> > level paging in DXE
> > > long mode
> > >
> > > Hi Ray,
> > >
> > > Given that there may be register definitions for
> > other CPUs or devices
> > > added to MdePkg in the future, should an extra
> > directory level be
> > > added? Doing that would break source compatibility
> > for existing
> > > components that use #include <Register/Cpuid.h> from
> > UefiCpuPkg. We
> > > could keep Cpuid.h in UefiCpuPkg, and it could be a
> > #include of the
> > > new Cpuid.h file in the MdePkg in the extended path.
> > >
> > > Also, should CpuId.h be included from BaseLib.h
> > inside:
> > >
> > > #if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
> > >
> > > This would make all CPUID related register
> > definitions available to
> > > components the needs BaseLib to call
> > > AsmCpuId() or AsmCpuIdEx()?
> > >
> > > We could also move the CRx, FLAGS/EFLAGS/descriptor
> > structures out of
> > > BaseLib.h into include files that are peers to
> > Cpuid.h and could be
> > > updated based on public spec updates.
> > >
> > > Thanks,
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io
> > > > [mailto:devel@edk2.groups.io] On Behalf Of Ni, Ray
> > > > Sent: Wednesday, July 24, 2019 3:00 AM
> > > > To: devel@edk2.groups.io
> > > > Subject: [edk2-devel] [PATCH V2 0/6] Support 5-
> > level paging in DXE
> > > > long mode
> > > >
> > > > v2:
> > > > Refined the patch according to reviewers' all
> > comments except:
> > > > 0A0h cannot be changed to A0h or build fails.
> > > > A big change in this patch is Cpuid.h is moved
> > from UefiCpuPkg to
> > > > MdePkg.
> > > > The move is based on real requirement when
> > certain modules that
> > > > cannot
> > > > depend on UefiCpuPkg but needs to reference
> > structures defined in
> > > > SDM.
> > > >
> > > > Ray Ni (6):
> > > > UefiCpuPkg/MpInitLib: Enable 5-level paging for
> > AP when BSP's
> > > > enabled
> > > > UefiCpuPkg/CpuDxe: Remove unnecessary macros
> > > > UefiCpuPkg/CpuDxe: Support parsing 5-level page
> > table
> > > > MdeModulePkg/DxeIpl: Introduce PCD
> > PcdUse5LevelPageTable
> > > > MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to
> > MdePkg
> > > > MdeModulePkg/DxeIpl: Create 5-level page table
> > for long mode
> > > >
> > > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf |
> > 1 +
> > > > .../Core/DxeIplPeim/X64/VirtualMemory.c |
> > 229
> > > > ++++++++++++------
> > > > MdeModulePkg/MdeModulePkg.dec |
> > 7 +
> > > > MdeModulePkg/MdeModulePkg.uni |
> > 8 +
> > > > .../Include/Register/Cpuid.h |
> > 0
> > > > UefiCpuPkg/CpuDxe/CpuPageTable.c |
> > 59
> > > > +++--
> > > > UefiCpuPkg/CpuDxe/CpuPageTable.h |
> > 3 +-
> > > > UefiCpuPkg/Library/MpInitLib/MpLib.c |
> > 13 +
> > > > UefiCpuPkg/Library/MpInitLib/MpLib.h |
> > 6 +-
> > > > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc |
> > 3 +-
> > > > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm |
> > 14 +-
> > > > 11 files changed, 243 insertions(+), 100
> > deletions(-) rename
> > > > {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h
> > > > (100%)
> > > >
> > > > --
> > > > 2.21.0.windows.1
> > > >
> > > >
> > > >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode
2019-07-25 5:39 ` Ni, Ray
@ 2019-07-25 15:55 ` Michael D Kinney
2019-07-25 23:42 ` Ni, Ray
0 siblings, 1 reply; 21+ messages in thread
From: Michael D Kinney @ 2019-07-25 15:55 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io, Kinney, Michael D
Ray,
I prefer to add a Vendor to the path based
on the vendor who published the specification.
Adding the vendor to the path will allow
include files to be added in the future with
clear names.
The UefiCpuPkg is an example of a poor choice
for the original Cpuid.h file. It should have
been in a vendor specific directory from the
beginning. The AMD one is correct in that
package.
I would also like to consider moving more of
The ARM and AARCH64 content into MdePkg to help
with package dependencies.
If we really want only a single directory,
then another option is to add the vendor
name to the include filename.
Mike
> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, July 24, 2019 10:40 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
> level paging in DXE long mode
>
> Mike,
> All the CPUID definitions also applies to AMD
> processors.
> There are two Cpuid.h in UefiCpuPkg today.
> 1. UefiCpuPkg/Include/Register/Cpuid.h
> 2. UefiCpuPkg/Include/Register/Amd/Cpuid.h
>
> The second one contains additional structures needed by
> AMD processor.
> But first one also applies to AMD processor.
>
> Can we just put Cpuid.h under MdePkg/Include/Register/
> because they are common to both Intel and AMD?
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Thursday, July 25, 2019 8:52 AM
> > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io;
> Kinney, Michael
> > D <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
> level paging in DXE
> > long mode
> >
> > Ray,
> >
> > I think the use of Include/Register is good for this
> type of content.
> > But we may need a Vendor directory.
> >
> > The general form would be:
> >
> > Include/Register/<Vendor>/<RegisterFile>.h
> >
> > Since the definitions being discussed here are from
> an Intel public
> > document, perhaps the path should be:
> >
> > Include/Register/Intel/Cpuid.h
> >
> > Thanks,
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Ni, Ray
> > > Sent: Wednesday, July 24, 2019 5:46 PM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > devel@edk2.groups.io
> > > Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
> level paging in
> > > DXE long mode
> > >
> > > Mike,
> > > Are you suggesting that
> > > 1. Copy Cpuid.h in MdePkg/Include/IndustryStandard/
> 2.
> > > Change UefiCpuPkg/Include/Register/Cpuid.h to just
> include
> > > <IndustryStandard/Cpuid.h>
> > >
> > > It looks like a potential issue that there are two
> "Cpuid.h" public
> > > header file in different folders.
> > > But given the include pattern used:
> > > "<Register/Cpuid.h>" VS
> "<IndustryStandard/Cpuid.h>", the risk
> > > people may include wrong file or compilers don't
> know which file to
> > > use is zero.
> > >
> > > Is that what you think?
> > >
> > > Thanks,
> > > Ray
> > >
> > > > -----Original Message-----
> > > > From: Kinney, Michael D
> > > > Sent: Thursday, July 25, 2019 1:23 AM
> > > > To: devel@edk2.groups.io; Ni, Ray
> <ray.ni@intel.com>;
> > > Kinney, Michael
> > > > D <michael.d.kinney@intel.com>
> > > > Subject: RE: [edk2-devel] [PATCH V2 0/6] Support
> 5-
> > > level paging in DXE
> > > > long mode
> > > >
> > > > Hi Ray,
> > > >
> > > > Given that there may be register definitions for
> > > other CPUs or devices
> > > > added to MdePkg in the future, should an extra
> > > directory level be
> > > > added? Doing that would break source
> compatibility
> > > for existing
> > > > components that use #include <Register/Cpuid.h>
> from
> > > UefiCpuPkg. We
> > > > could keep Cpuid.h in UefiCpuPkg, and it could be
> a
> > > #include of the
> > > > new Cpuid.h file in the MdePkg in the extended
> path.
> > > >
> > > > Also, should CpuId.h be included from BaseLib.h
> > > inside:
> > > >
> > > > #if defined (MDE_CPU_IA32) || defined
> (MDE_CPU_X64)
> > > >
> > > > This would make all CPUID related register
> > > definitions available to
> > > > components the needs BaseLib to call
> > > > AsmCpuId() or AsmCpuIdEx()?
> > > >
> > > > We could also move the CRx,
> FLAGS/EFLAGS/descriptor
> > > structures out of
> > > > BaseLib.h into include files that are peers to
> > > Cpuid.h and could be
> > > > updated based on public spec updates.
> > > >
> > > > Thanks,
> > > >
> > > > Mike
> > > >
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io
> > > > > [mailto:devel@edk2.groups.io] On Behalf Of Ni,
> Ray
> > > > > Sent: Wednesday, July 24, 2019 3:00 AM
> > > > > To: devel@edk2.groups.io
> > > > > Subject: [edk2-devel] [PATCH V2 0/6] Support 5-
> > > level paging in DXE
> > > > > long mode
> > > > >
> > > > > v2:
> > > > > Refined the patch according to reviewers'
> all
> > > comments except:
> > > > > 0A0h cannot be changed to A0h or build
> fails.
> > > > > A big change in this patch is Cpuid.h is
> moved
> > > from UefiCpuPkg to
> > > > > MdePkg.
> > > > > The move is based on real requirement when
> > > certain modules that
> > > > > cannot
> > > > > depend on UefiCpuPkg but needs to reference
> > > structures defined in
> > > > > SDM.
> > > > >
> > > > > Ray Ni (6):
> > > > > UefiCpuPkg/MpInitLib: Enable 5-level paging
> for
> > > AP when BSP's
> > > > > enabled
> > > > > UefiCpuPkg/CpuDxe: Remove unnecessary macros
> > > > > UefiCpuPkg/CpuDxe: Support parsing 5-level
> page
> > > table
> > > > > MdeModulePkg/DxeIpl: Introduce PCD
> > > PcdUse5LevelPageTable
> > > > > MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg
> to
> > > MdePkg
> > > > > MdeModulePkg/DxeIpl: Create 5-level page
> table
> > > for long mode
> > > > >
> > > > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> |
> > > 1 +
> > > > > .../Core/DxeIplPeim/X64/VirtualMemory.c
> |
> > > 229
> > > > > ++++++++++++------
> > > > > MdeModulePkg/MdeModulePkg.dec
> |
> > > 7 +
> > > > > MdeModulePkg/MdeModulePkg.uni
> |
> > > 8 +
> > > > > .../Include/Register/Cpuid.h
> |
> > > 0
> > > > > UefiCpuPkg/CpuDxe/CpuPageTable.c
> |
> > > 59
> > > > > +++--
> > > > > UefiCpuPkg/CpuDxe/CpuPageTable.h
> |
> > > 3 +-
> > > > > UefiCpuPkg/Library/MpInitLib/MpLib.c
> |
> > > 13 +
> > > > > UefiCpuPkg/Library/MpInitLib/MpLib.h
> |
> > > 6 +-
> > > > > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> |
> > > 3 +-
> > > > > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> |
> > > 14 +-
> > > > > 11 files changed, 243 insertions(+), 100
> > > deletions(-) rename
> > > > > {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h
> > > > > (100%)
> > > > >
> > > > > --
> > > > > 2.21.0.windows.1
> > > > >
> > > > >
> > > > >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode
2019-07-25 15:55 ` Michael D Kinney
@ 2019-07-25 23:42 ` Ni, Ray
2019-07-26 18:17 ` Michael D Kinney
0 siblings, 1 reply; 21+ messages in thread
From: Ni, Ray @ 2019-07-25 23:42 UTC (permalink / raw)
To: Kinney, Michael D; +Cc: devel@edk2.groups.io
Having the vendor name for cpu may cause confusion to customers.
AMD customer may found even their code is running in AMD processors Intel/Cpuid.h is still included.
It may also be possible that Intel platform code has to include AMD/Cpuid.h.
The CPU is different from other hardwares. It is ok that two PCIE cards from different vendors exist in the same platform. But not Ok for CPU.
Not sure if I am over concerned.
> On Jul 25, 2019, at 11:55 PM, Kinney, Michael D <michael.d.kinney@intel.com> wrote:
>
> Ray,
>
> I prefer to add a Vendor to the path based
> on the vendor who published the specification.
> Adding the vendor to the path will allow
> include files to be added in the future with
> clear names.
>
> The UefiCpuPkg is an example of a poor choice
> for the original Cpuid.h file. It should have
> been in a vendor specific directory from the
> beginning. The AMD one is correct in that
> package.
>
> I would also like to consider moving more of
> The ARM and AARCH64 content into MdePkg to help
> with package dependencies.
>
> If we really want only a single directory,
> then another option is to add the vendor
> name to the include filename.
>
> Mike
>
>> -----Original Message-----
>> From: Ni, Ray
>> Sent: Wednesday, July 24, 2019 10:40 PM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>;
>> devel@edk2.groups.io
>> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
>> level paging in DXE long mode
>>
>> Mike,
>> All the CPUID definitions also applies to AMD
>> processors.
>> There are two Cpuid.h in UefiCpuPkg today.
>> 1. UefiCpuPkg/Include/Register/Cpuid.h
>> 2. UefiCpuPkg/Include/Register/Amd/Cpuid.h
>>
>> The second one contains additional structures needed by
>> AMD processor.
>> But first one also applies to AMD processor.
>>
>> Can we just put Cpuid.h under MdePkg/Include/Register/
>> because they are common to both Intel and AMD?
>>
>> Thanks,
>> Ray
>>
>>> -----Original Message-----
>>> From: Kinney, Michael D
>>> Sent: Thursday, July 25, 2019 8:52 AM
>>> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io;
>> Kinney, Michael
>>> D <michael.d.kinney@intel.com>
>>> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
>> level paging in DXE
>>> long mode
>>>
>>> Ray,
>>>
>>> I think the use of Include/Register is good for this
>> type of content.
>>> But we may need a Vendor directory.
>>>
>>> The general form would be:
>>>
>>> Include/Register/<Vendor>/<RegisterFile>.h
>>>
>>> Since the definitions being discussed here are from
>> an Intel public
>>> document, perhaps the path should be:
>>>
>>> Include/Register/Intel/Cpuid.h
>>>
>>> Thanks,
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: Ni, Ray
>>>> Sent: Wednesday, July 24, 2019 5:46 PM
>>>> To: Kinney, Michael D <michael.d.kinney@intel.com>;
>>>> devel@edk2.groups.io
>>>> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
>> level paging in
>>>> DXE long mode
>>>>
>>>> Mike,
>>>> Are you suggesting that
>>>> 1. Copy Cpuid.h in MdePkg/Include/IndustryStandard/
>> 2.
>>>> Change UefiCpuPkg/Include/Register/Cpuid.h to just
>> include
>>>> <IndustryStandard/Cpuid.h>
>>>>
>>>> It looks like a potential issue that there are two
>> "Cpuid.h" public
>>>> header file in different folders.
>>>> But given the include pattern used:
>>>> "<Register/Cpuid.h>" VS
>> "<IndustryStandard/Cpuid.h>", the risk
>>>> people may include wrong file or compilers don't
>> know which file to
>>>> use is zero.
>>>>
>>>> Is that what you think?
>>>>
>>>> Thanks,
>>>> Ray
>>>>
>>>>> -----Original Message-----
>>>>> From: Kinney, Michael D
>>>>> Sent: Thursday, July 25, 2019 1:23 AM
>>>>> To: devel@edk2.groups.io; Ni, Ray
>> <ray.ni@intel.com>;
>>>> Kinney, Michael
>>>>> D <michael.d.kinney@intel.com>
>>>>> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support
>> 5-
>>>> level paging in DXE
>>>>> long mode
>>>>>
>>>>> Hi Ray,
>>>>>
>>>>> Given that there may be register definitions for
>>>> other CPUs or devices
>>>>> added to MdePkg in the future, should an extra
>>>> directory level be
>>>>> added? Doing that would break source
>> compatibility
>>>> for existing
>>>>> components that use #include <Register/Cpuid.h>
>> from
>>>> UefiCpuPkg. We
>>>>> could keep Cpuid.h in UefiCpuPkg, and it could be
>> a
>>>> #include of the
>>>>> new Cpuid.h file in the MdePkg in the extended
>> path.
>>>>>
>>>>> Also, should CpuId.h be included from BaseLib.h
>>>> inside:
>>>>>
>>>>> #if defined (MDE_CPU_IA32) || defined
>> (MDE_CPU_X64)
>>>>>
>>>>> This would make all CPUID related register
>>>> definitions available to
>>>>> components the needs BaseLib to call
>>>>> AsmCpuId() or AsmCpuIdEx()?
>>>>>
>>>>> We could also move the CRx,
>> FLAGS/EFLAGS/descriptor
>>>> structures out of
>>>>> BaseLib.h into include files that are peers to
>>>> Cpuid.h and could be
>>>>> updated based on public spec updates.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Mike
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: devel@edk2.groups.io
>>>>>> [mailto:devel@edk2.groups.io] On Behalf Of Ni,
>> Ray
>>>>>> Sent: Wednesday, July 24, 2019 3:00 AM
>>>>>> To: devel@edk2.groups.io
>>>>>> Subject: [edk2-devel] [PATCH V2 0/6] Support 5-
>>>> level paging in DXE
>>>>>> long mode
>>>>>>
>>>>>> v2:
>>>>>> Refined the patch according to reviewers'
>> all
>>>> comments except:
>>>>>> 0A0h cannot be changed to A0h or build
>> fails.
>>>>>> A big change in this patch is Cpuid.h is
>> moved
>>>> from UefiCpuPkg to
>>>>>> MdePkg.
>>>>>> The move is based on real requirement when
>>>> certain modules that
>>>>>> cannot
>>>>>> depend on UefiCpuPkg but needs to reference
>>>> structures defined in
>>>>>> SDM.
>>>>>>
>>>>>> Ray Ni (6):
>>>>>> UefiCpuPkg/MpInitLib: Enable 5-level paging
>> for
>>>> AP when BSP's
>>>>>> enabled
>>>>>> UefiCpuPkg/CpuDxe: Remove unnecessary macros
>>>>>> UefiCpuPkg/CpuDxe: Support parsing 5-level
>> page
>>>> table
>>>>>> MdeModulePkg/DxeIpl: Introduce PCD
>>>> PcdUse5LevelPageTable
>>>>>> MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg
>> to
>>>> MdePkg
>>>>>> MdeModulePkg/DxeIpl: Create 5-level page
>> table
>>>> for long mode
>>>>>>
>>>>>> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>> |
>>>> 1 +
>>>>>> .../Core/DxeIplPeim/X64/VirtualMemory.c
>> |
>>>> 229
>>>>>> ++++++++++++------
>>>>>> MdeModulePkg/MdeModulePkg.dec
>> |
>>>> 7 +
>>>>>> MdeModulePkg/MdeModulePkg.uni
>> |
>>>> 8 +
>>>>>> .../Include/Register/Cpuid.h
>> |
>>>> 0
>>>>>> UefiCpuPkg/CpuDxe/CpuPageTable.c
>> |
>>>> 59
>>>>>> +++--
>>>>>> UefiCpuPkg/CpuDxe/CpuPageTable.h
>> |
>>>> 3 +-
>>>>>> UefiCpuPkg/Library/MpInitLib/MpLib.c
>> |
>>>> 13 +
>>>>>> UefiCpuPkg/Library/MpInitLib/MpLib.h
>> |
>>>> 6 +-
>>>>>> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>> |
>>>> 3 +-
>>>>>> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
>> |
>>>> 14 +-
>>>>>> 11 files changed, 243 insertions(+), 100
>>>> deletions(-) rename
>>>>>> {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h
>>>>>> (100%)
>>>>>>
>>>>>> --
>>>>>> 2.21.0.windows.1
>>>>>>
>>>>>>
>>>>>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode
2019-07-24 10:00 [PATCH V2 0/6] Support 5-level paging in DXE long mode Ni, Ray
` (6 preceding siblings ...)
2019-07-24 17:23 ` [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE " Michael D Kinney
@ 2019-07-26 10:14 ` Laszlo Ersek
7 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2019-07-26 10:14 UTC (permalink / raw)
To: devel, ray.ni
On 07/24/19 12:00, Ni, Ray wrote:
> v2:
> Refined the patch according to reviewers' all comments except:
> 0A0h cannot be changed to A0h or build fails.
> A big change in this patch is Cpuid.h is moved from UefiCpuPkg to MdePkg.
> The move is based on real requirement when certain modules that cannot
> depend on UefiCpuPkg but needs to reference structures defined in SDM.
>
> Ray Ni (6):
> UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled
> UefiCpuPkg/CpuDxe: Remove unnecessary macros
> UefiCpuPkg/CpuDxe: Support parsing 5-level page table
> MdeModulePkg/DxeIpl: Introduce PCD PcdUse5LevelPageTable
> MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
> MdeModulePkg/DxeIpl: Create 5-level page table for long mode
>
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 +
> .../Core/DxeIplPeim/X64/VirtualMemory.c | 229 ++++++++++++------
> MdeModulePkg/MdeModulePkg.dec | 7 +
> MdeModulePkg/MdeModulePkg.uni | 8 +
> .../Include/Register/Cpuid.h | 0
> UefiCpuPkg/CpuDxe/CpuPageTable.c | 59 +++--
> UefiCpuPkg/CpuDxe/CpuPageTable.h | 3 +-
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 6 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 3 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 14 +-
> 11 files changed, 243 insertions(+), 100 deletions(-)
> rename {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h (100%)
>
I'd like to regression-test this series once the reviews around it have
converged. My current understanding is that a v3 is needed, so I plan to
wait for v3. If that turns out not to be the case, please ping me
separately.
Thank you
Laszlo
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode
2019-07-25 23:42 ` Ni, Ray
@ 2019-07-26 18:17 ` Michael D Kinney
0 siblings, 0 replies; 21+ messages in thread
From: Michael D Kinney @ 2019-07-26 18:17 UTC (permalink / raw)
To: Ni, Ray, Kinney, Michael D; +Cc: devel@edk2.groups.io
Ray,
The .h files represent information from specifications and
components that include the include files do so because
their source code depends on definitions associated with
those specifications. It does not represent what CPUs
are present in the platform. It may represent what
CPUs the module/lib supports.
Mike
> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, July 25, 2019 4:42 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: devel@edk2.groups.io
> Subject: Re: [edk2-devel] [PATCH V2 0/6] Support 5-
> level paging in DXE long mode
>
> Having the vendor name for cpu may cause confusion to
> customers.
> AMD customer may found even their code is running in
> AMD processors Intel/Cpuid.h is still included.
> It may also be possible that Intel platform code has to
> include AMD/Cpuid.h.
>
> The CPU is different from other hardwares. It is ok
> that two PCIE cards from different vendors exist in the
> same platform. But not Ok for CPU.
> Not sure if I am over concerned.
>
> Sent from my iPhone
>
> > On Jul 25, 2019, at 11:55 PM, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
> >
> > Ray,
> >
> > I prefer to add a Vendor to the path based on the
> vendor who published
> > the specification.
> > Adding the vendor to the path will allow include
> files to be added in
> > the future with clear names.
> >
> > The UefiCpuPkg is an example of a poor choice for the
> original Cpuid.h
> > file. It should have been in a vendor specific
> directory from the
> > beginning. The AMD one is correct in that package.
> >
> > I would also like to consider moving more of The ARM
> and AARCH64
> > content into MdePkg to help with package
> dependencies.
> >
> > If we really want only a single directory, then
> another option is to
> > add the vendor name to the include filename.
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Ni, Ray
> >> Sent: Wednesday, July 24, 2019 10:40 PM
> >> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> >> devel@edk2.groups.io
> >> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
> level paging in
> >> DXE long mode
> >>
> >> Mike,
> >> All the CPUID definitions also applies to AMD
> processors.
> >> There are two Cpuid.h in UefiCpuPkg today.
> >> 1. UefiCpuPkg/Include/Register/Cpuid.h
> >> 2. UefiCpuPkg/Include/Register/Amd/Cpuid.h
> >>
> >> The second one contains additional structures needed
> by AMD
> >> processor.
> >> But first one also applies to AMD processor.
> >>
> >> Can we just put Cpuid.h under
> MdePkg/Include/Register/ because they
> >> are common to both Intel and AMD?
> >>
> >> Thanks,
> >> Ray
> >>
> >>> -----Original Message-----
> >>> From: Kinney, Michael D
> >>> Sent: Thursday, July 25, 2019 8:52 AM
> >>> To: Ni, Ray <ray.ni@intel.com>;
> devel@edk2.groups.io;
> >> Kinney, Michael
> >>> D <michael.d.kinney@intel.com>
> >>> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support 5-
> >> level paging in DXE
> >>> long mode
> >>>
> >>> Ray,
> >>>
> >>> I think the use of Include/Register is good for
> this
> >> type of content.
> >>> But we may need a Vendor directory.
> >>>
> >>> The general form would be:
> >>>
> >>> Include/Register/<Vendor>/<RegisterFile>.h
> >>>
> >>> Since the definitions being discussed here are from
> >> an Intel public
> >>> document, perhaps the path should be:
> >>>
> >>> Include/Register/Intel/Cpuid.h
> >>>
> >>> Thanks,
> >>>
> >>> Mike
> >>>
> >>>> -----Original Message-----
> >>>> From: Ni, Ray
> >>>> Sent: Wednesday, July 24, 2019 5:46 PM
> >>>> To: Kinney, Michael D
> <michael.d.kinney@intel.com>;
> >>>> devel@edk2.groups.io
> >>>> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support
> 5-
> >> level paging in
> >>>> DXE long mode
> >>>>
> >>>> Mike,
> >>>> Are you suggesting that
> >>>> 1. Copy Cpuid.h in
> MdePkg/Include/IndustryStandard/
> >> 2.
> >>>> Change UefiCpuPkg/Include/Register/Cpuid.h to just
> >> include
> >>>> <IndustryStandard/Cpuid.h>
> >>>>
> >>>> It looks like a potential issue that there are two
> >> "Cpuid.h" public
> >>>> header file in different folders.
> >>>> But given the include pattern used:
> >>>> "<Register/Cpuid.h>" VS
> >> "<IndustryStandard/Cpuid.h>", the risk
> >>>> people may include wrong file or compilers don't
> >> know which file to
> >>>> use is zero.
> >>>>
> >>>> Is that what you think?
> >>>>
> >>>> Thanks,
> >>>> Ray
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Kinney, Michael D
> >>>>> Sent: Thursday, July 25, 2019 1:23 AM
> >>>>> To: devel@edk2.groups.io; Ni, Ray
> >> <ray.ni@intel.com>;
> >>>> Kinney, Michael
> >>>>> D <michael.d.kinney@intel.com>
> >>>>> Subject: RE: [edk2-devel] [PATCH V2 0/6] Support
> >> 5-
> >>>> level paging in DXE
> >>>>> long mode
> >>>>>
> >>>>> Hi Ray,
> >>>>>
> >>>>> Given that there may be register definitions for
> >>>> other CPUs or devices
> >>>>> added to MdePkg in the future, should an extra
> >>>> directory level be
> >>>>> added? Doing that would break source
> >> compatibility
> >>>> for existing
> >>>>> components that use #include <Register/Cpuid.h>
> >> from
> >>>> UefiCpuPkg. We
> >>>>> could keep Cpuid.h in UefiCpuPkg, and it could be
> >> a
> >>>> #include of the
> >>>>> new Cpuid.h file in the MdePkg in the extended
> >> path.
> >>>>>
> >>>>> Also, should CpuId.h be included from BaseLib.h
> >>>> inside:
> >>>>>
> >>>>> #if defined (MDE_CPU_IA32) || defined
> >> (MDE_CPU_X64)
> >>>>>
> >>>>> This would make all CPUID related register
> >>>> definitions available to
> >>>>> components the needs BaseLib to call
> >>>>> AsmCpuId() or AsmCpuIdEx()?
> >>>>>
> >>>>> We could also move the CRx,
> >> FLAGS/EFLAGS/descriptor
> >>>> structures out of
> >>>>> BaseLib.h into include files that are peers to
> >>>> Cpuid.h and could be
> >>>>> updated based on public spec updates.
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Mike
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: devel@edk2.groups.io
> >>>>>> [mailto:devel@edk2.groups.io] On Behalf Of Ni,
> >> Ray
> >>>>>> Sent: Wednesday, July 24, 2019 3:00 AM
> >>>>>> To: devel@edk2.groups.io
> >>>>>> Subject: [edk2-devel] [PATCH V2 0/6] Support 5-
> >>>> level paging in DXE
> >>>>>> long mode
> >>>>>>
> >>>>>> v2:
> >>>>>> Refined the patch according to reviewers'
> >> all
> >>>> comments except:
> >>>>>> 0A0h cannot be changed to A0h or build
> >> fails.
> >>>>>> A big change in this patch is Cpuid.h is
> >> moved
> >>>> from UefiCpuPkg to
> >>>>>> MdePkg.
> >>>>>> The move is based on real requirement when
> >>>> certain modules that
> >>>>>> cannot
> >>>>>> depend on UefiCpuPkg but needs to reference
> >>>> structures defined in
> >>>>>> SDM.
> >>>>>>
> >>>>>> Ray Ni (6):
> >>>>>> UefiCpuPkg/MpInitLib: Enable 5-level paging
> >> for
> >>>> AP when BSP's
> >>>>>> enabled
> >>>>>> UefiCpuPkg/CpuDxe: Remove unnecessary macros
> >>>>>> UefiCpuPkg/CpuDxe: Support parsing 5-level
> >> page
> >>>> table
> >>>>>> MdeModulePkg/DxeIpl: Introduce PCD
> >>>> PcdUse5LevelPageTable
> >>>>>> MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg
> >> to
> >>>> MdePkg
> >>>>>> MdeModulePkg/DxeIpl: Create 5-level page
> >> table
> >>>> for long mode
> >>>>>>
> >>>>>> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> >> |
> >>>> 1 +
> >>>>>> .../Core/DxeIplPeim/X64/VirtualMemory.c
> >> |
> >>>> 229
> >>>>>> ++++++++++++------
> >>>>>> MdeModulePkg/MdeModulePkg.dec
> >> |
> >>>> 7 +
> >>>>>> MdeModulePkg/MdeModulePkg.uni
> >> |
> >>>> 8 +
> >>>>>> .../Include/Register/Cpuid.h
> >> |
> >>>> 0
> >>>>>> UefiCpuPkg/CpuDxe/CpuPageTable.c
> >> |
> >>>> 59
> >>>>>> +++--
> >>>>>> UefiCpuPkg/CpuDxe/CpuPageTable.h
> >> |
> >>>> 3 +-
> >>>>>> UefiCpuPkg/Library/MpInitLib/MpLib.c
> >> |
> >>>> 13 +
> >>>>>> UefiCpuPkg/Library/MpInitLib/MpLib.h
> >> |
> >>>> 6 +-
> >>>>>> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> >> |
> >>>> 3 +-
> >>>>>> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> >> |
> >>>> 14 +-
> >>>>>> 11 files changed, 243 insertions(+), 100
> >>>> deletions(-) rename
> >>>>>> {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h
> >>>>>> (100%)
> >>>>>>
> >>>>>> --
> >>>>>> 2.21.0.windows.1
> >>>>>>
> >>>>>>
> >>>>>>
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
2019-07-24 10:00 ` [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg Ni, Ray
@ 2019-07-30 3:29 ` Liming Gao
2019-07-30 3:42 ` Michael D Kinney
0 siblings, 1 reply; 21+ messages in thread
From: Liming Gao @ 2019-07-30 3:29 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io; +Cc: Kinney, Michael D, Dong, Eric, Laszlo Ersek
The change is good to me. Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, July 24, 2019 6:00 PM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2008
>
> MdeModulePkg/DxeIpl needs to get CPUID output for CPU
> 5-level paging capability detection.
>
> In order to use the macros/structures defined in
> UefiCpuPkg/Include/Register/Cpuid.h, the patch moves
> Cpuid.h to MdePkg/Include/Register/ directory.
>
> Because almost every module depends on MdePkg.dec in its
> INF file, the move almost has no impact.
>
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
> {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> rename {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h (100%)
>
> diff --git a/UefiCpuPkg/Include/Register/Cpuid.h b/MdePkg/Include/Register/Cpuid.h
> similarity index 100%
> rename from UefiCpuPkg/Include/Register/Cpuid.h
> rename to MdePkg/Include/Register/Cpuid.h
> --
> 2.21.0.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
2019-07-30 3:29 ` Liming Gao
@ 2019-07-30 3:42 ` Michael D Kinney
2019-07-30 3:49 ` Liming Gao
0 siblings, 1 reply; 21+ messages in thread
From: Michael D Kinney @ 2019-07-30 3:42 UTC (permalink / raw)
To: Gao, Liming, Ni, Ray, devel@edk2.groups.io, Kinney, Michael D
Cc: Dong, Eric, Laszlo Ersek
Liming,
There is an unresolved discussion on the location of this
file in the MdePkg. I prefer a vendor name in the path.
MdePkg/Include/Register/Intel/Cpuid.h
Also, after this file is moved to MdePkg, we can enter
new BZs to update all the modules that call AsmCpuid and
AsmCpuidEx to use the defines and structures from Cpuid.h.
Mike
> -----Original Message-----
> From: Gao, Liming
> Sent: Monday, July 29, 2019 8:29 PM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> Cpuid.h from UefiCpuPkg to MdePkg
>
> The change is good to me. Reviewed-by: Liming Gao
> <liming.gao@intel.com>
>
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Wednesday, July 24, 2019 6:00 PM
> > To: devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Gao, Liming
> > <liming.gao@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Laszlo Ersek
> > <lersek@redhat.com>
> > Subject: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h
> from UefiCpuPkg
> > to MdePkg
> >
> > REF:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2008
> >
> > MdeModulePkg/DxeIpl needs to get CPUID output for CPU
> 5-level paging
> > capability detection.
> >
> > In order to use the macros/structures defined in
> > UefiCpuPkg/Include/Register/Cpuid.h, the patch moves
> Cpuid.h to
> > MdePkg/Include/Register/ directory.
> >
> > Because almost every module depends on MdePkg.dec in
> its INF file, the
> > move almost has no impact.
> >
> > Signed-off-by: Ray Ni <ray.ni@intel.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > ---
> > {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h | 0
> > 1 file changed, 0 insertions(+), 0 deletions(-)
> rename {UefiCpuPkg
> > => MdePkg}/Include/Register/Cpuid.h (100%)
> >
> > diff --git a/UefiCpuPkg/Include/Register/Cpuid.h
> > b/MdePkg/Include/Register/Cpuid.h similarity index
> 100% rename from
> > UefiCpuPkg/Include/Register/Cpuid.h
> > rename to MdePkg/Include/Register/Cpuid.h
> > --
> > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
2019-07-30 3:42 ` Michael D Kinney
@ 2019-07-30 3:49 ` Liming Gao
2019-07-30 16:30 ` Michael D Kinney
0 siblings, 1 reply; 21+ messages in thread
From: Liming Gao @ 2019-07-30 3:49 UTC (permalink / raw)
To: Kinney, Michael D, Ni, Ray, devel@edk2.groups.io; +Cc: Dong, Eric, Laszlo Ersek
Mike:
OK. If so, the consumer code also need to obviously include vendor header file, such as #include <Register/Intel/Cpuid.h>. Right?
Thanks
Liming
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Tuesday, July 30, 2019 11:42 AM
> To: Gao, Liming <liming.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
>
> Liming,
>
> There is an unresolved discussion on the location of this
> file in the MdePkg. I prefer a vendor name in the path.
>
> MdePkg/Include/Register/Intel/Cpuid.h
>
> Also, after this file is moved to MdePkg, we can enter
> new BZs to update all the modules that call AsmCpuid and
> AsmCpuidEx to use the defines and structures from Cpuid.h.
>
> Mike
>
> > -----Original Message-----
> > From: Gao, Liming
> > Sent: Monday, July 29, 2019 8:29 PM
> > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> > <lersek@redhat.com>
> > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> > Cpuid.h from UefiCpuPkg to MdePkg
> >
> > The change is good to me. Reviewed-by: Liming Gao
> > <liming.gao@intel.com>
> >
> > > -----Original Message-----
> > > From: Ni, Ray
> > > Sent: Wednesday, July 24, 2019 6:00 PM
> > > To: devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Gao, Liming
> > > <liming.gao@intel.com>; Dong, Eric
> > <eric.dong@intel.com>; Laszlo Ersek
> > > <lersek@redhat.com>
> > > Subject: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h
> > from UefiCpuPkg
> > > to MdePkg
> > >
> > > REF:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2008
> > >
> > > MdeModulePkg/DxeIpl needs to get CPUID output for CPU
> > 5-level paging
> > > capability detection.
> > >
> > > In order to use the macros/structures defined in
> > > UefiCpuPkg/Include/Register/Cpuid.h, the patch moves
> > Cpuid.h to
> > > MdePkg/Include/Register/ directory.
> > >
> > > Because almost every module depends on MdePkg.dec in
> > its INF file, the
> > > move almost has no impact.
> > >
> > > Signed-off-by: Ray Ni <ray.ni@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > ---
> > > {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h | 0
> > > 1 file changed, 0 insertions(+), 0 deletions(-)
> > rename {UefiCpuPkg
> > > => MdePkg}/Include/Register/Cpuid.h (100%)
> > >
> > > diff --git a/UefiCpuPkg/Include/Register/Cpuid.h
> > > b/MdePkg/Include/Register/Cpuid.h similarity index
> > 100% rename from
> > > UefiCpuPkg/Include/Register/Cpuid.h
> > > rename to MdePkg/Include/Register/Cpuid.h
> > > --
> > > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
2019-07-30 3:49 ` Liming Gao
@ 2019-07-30 16:30 ` Michael D Kinney
2019-08-01 2:26 ` Ni, Ray
0 siblings, 1 reply; 21+ messages in thread
From: Michael D Kinney @ 2019-07-30 16:30 UTC (permalink / raw)
To: Gao, Liming, Ni, Ray, devel@edk2.groups.io, Kinney, Michael D
Cc: Dong, Eric, Laszlo Ersek
Liming,
Yes. That would be the correct include statement for
a module/lib that depends on MdePkg.
I know we have modules/libs that use the UefiCpuPkg
and use #include <Register/Cpuid.h>. If we want to
provide compatibility with all consumers of the UefiCpuPkg,
the Cpuid.h file in the UefiCpuPkg can updated to simply
include <Register/Intel/Cpuid.h> from MdePkg. We could
also choose to update those consumers to use the new path
in the MdePkg.
Mike
> -----Original Message-----
> From: Gao, Liming
> Sent: Monday, July 29, 2019 8:49 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Ni,
> Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> Cpuid.h from UefiCpuPkg to MdePkg
>
> Mike:
> OK. If so, the consumer code also need to obviously
> include vendor header file, such as #include
> <Register/Intel/Cpuid.h>. Right?
>
> Thanks
> Liming
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Tuesday, July 30, 2019 11:42 AM
> > To: Gao, Liming <liming.gao@intel.com>; Ni, Ray
> <ray.ni@intel.com>;
> > devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> > Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> Cpuid.h from
> > UefiCpuPkg to MdePkg
> >
> > Liming,
> >
> > There is an unresolved discussion on the location of
> this file in the
> > MdePkg. I prefer a vendor name in the path.
> >
> > MdePkg/Include/Register/Intel/Cpuid.h
> >
> > Also, after this file is moved to MdePkg, we can
> enter new BZs to
> > update all the modules that call AsmCpuid and
> AsmCpuidEx to use the
> > defines and structures from Cpuid.h.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Gao, Liming
> > > Sent: Monday, July 29, 2019 8:29 PM
> > > To: Ni, Ray <ray.ni@intel.com>;
> devel@edk2.groups.io
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Dong, Eric
> > > <eric.dong@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> > > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> Cpuid.h from
> > > UefiCpuPkg to MdePkg
> > >
> > > The change is good to me. Reviewed-by: Liming Gao
> > > <liming.gao@intel.com>
> > >
> > > > -----Original Message-----
> > > > From: Ni, Ray
> > > > Sent: Wednesday, July 24, 2019 6:00 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > > Gao, Liming
> > > > <liming.gao@intel.com>; Dong, Eric
> > > <eric.dong@intel.com>; Laszlo Ersek
> > > > <lersek@redhat.com>
> > > > Subject: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> Cpuid.h
> > > from UefiCpuPkg
> > > > to MdePkg
> > > >
> > > > REF:
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2008
> > > >
> > > > MdeModulePkg/DxeIpl needs to get CPUID output for
> CPU
> > > 5-level paging
> > > > capability detection.
> > > >
> > > > In order to use the macros/structures defined in
> > > > UefiCpuPkg/Include/Register/Cpuid.h, the patch
> moves
> > > Cpuid.h to
> > > > MdePkg/Include/Register/ directory.
> > > >
> > > > Because almost every module depends on MdePkg.dec
> in
> > > its INF file, the
> > > > move almost has no impact.
> > > >
> > > > Signed-off-by: Ray Ni <ray.ni@intel.com>
> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > Cc: Eric Dong <eric.dong@intel.com>
> > > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > > ---
> > > > {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h
> | 0
> > > > 1 file changed, 0 insertions(+), 0 deletions(-)
> > > rename {UefiCpuPkg
> > > > => MdePkg}/Include/Register/Cpuid.h (100%)
> > > >
> > > > diff --git a/UefiCpuPkg/Include/Register/Cpuid.h
> > > > b/MdePkg/Include/Register/Cpuid.h similarity
> index
> > > 100% rename from
> > > > UefiCpuPkg/Include/Register/Cpuid.h
> > > > rename to MdePkg/Include/Register/Cpuid.h
> > > > --
> > > > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
2019-07-30 16:30 ` Michael D Kinney
@ 2019-08-01 2:26 ` Ni, Ray
2019-08-01 8:15 ` Michael D Kinney
0 siblings, 1 reply; 21+ messages in thread
From: Ni, Ray @ 2019-08-01 2:26 UTC (permalink / raw)
To: Kinney, Michael D, Gao, Liming, devel@edk2.groups.io
Cc: Dong, Eric, Laszlo Ersek
Mike,
Thanks for the time for offline discussion.
I agree with the idea to have an Intel/Cpuid.h in MdePkg/Include/Register/ directory.
Do you prefer to move Amd/Cpuid.h to MdePkg/Include/Register/ in my V3 patch?
Thanks,
Ray
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, July 31, 2019 12:31 AM
> To: Gao, Liming <liming.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
>
> Liming,
>
> Yes. That would be the correct include statement for
> a module/lib that depends on MdePkg.
>
> I know we have modules/libs that use the UefiCpuPkg
> and use #include <Register/Cpuid.h>. If we want to
> provide compatibility with all consumers of the UefiCpuPkg,
> the Cpuid.h file in the UefiCpuPkg can updated to simply
> include <Register/Intel/Cpuid.h> from MdePkg. We could
> also choose to update those consumers to use the new path
> in the MdePkg.
>
> Mike
>
> > -----Original Message-----
> > From: Gao, Liming
> > Sent: Monday, July 29, 2019 8:49 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>; Ni,
> > Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> > <lersek@redhat.com>
> > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> > Cpuid.h from UefiCpuPkg to MdePkg
> >
> > Mike:
> > OK. If so, the consumer code also need to obviously
> > include vendor header file, such as #include
> > <Register/Intel/Cpuid.h>. Right?
> >
> > Thanks
> > Liming
> > > -----Original Message-----
> > > From: Kinney, Michael D
> > > Sent: Tuesday, July 30, 2019 11:42 AM
> > > To: Gao, Liming <liming.gao@intel.com>; Ni, Ray
> > <ray.ni@intel.com>;
> > > devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > > Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> > <lersek@redhat.com>
> > > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> > Cpuid.h from
> > > UefiCpuPkg to MdePkg
> > >
> > > Liming,
> > >
> > > There is an unresolved discussion on the location of
> > this file in the
> > > MdePkg. I prefer a vendor name in the path.
> > >
> > > MdePkg/Include/Register/Intel/Cpuid.h
> > >
> > > Also, after this file is moved to MdePkg, we can
> > enter new BZs to
> > > update all the modules that call AsmCpuid and
> > AsmCpuidEx to use the
> > > defines and structures from Cpuid.h.
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: Gao, Liming
> > > > Sent: Monday, July 29, 2019 8:29 PM
> > > > To: Ni, Ray <ray.ni@intel.com>;
> > devel@edk2.groups.io
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Dong, Eric
> > > > <eric.dong@intel.com>; Laszlo Ersek
> > <lersek@redhat.com>
> > > > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> > Cpuid.h from
> > > > UefiCpuPkg to MdePkg
> > > >
> > > > The change is good to me. Reviewed-by: Liming Gao
> > > > <liming.gao@intel.com>
> > > >
> > > > > -----Original Message-----
> > > > > From: Ni, Ray
> > > > > Sent: Wednesday, July 24, 2019 6:00 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Kinney, Michael D
> > <michael.d.kinney@intel.com>;
> > > > Gao, Liming
> > > > > <liming.gao@intel.com>; Dong, Eric
> > > > <eric.dong@intel.com>; Laszlo Ersek
> > > > > <lersek@redhat.com>
> > > > > Subject: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> > Cpuid.h
> > > > from UefiCpuPkg
> > > > > to MdePkg
> > > > >
> > > > > REF:
> > > > https://bugzilla.tianocore.org/show_bug.cgi?id=2008
> > > > >
> > > > > MdeModulePkg/DxeIpl needs to get CPUID output for
> > CPU
> > > > 5-level paging
> > > > > capability detection.
> > > > >
> > > > > In order to use the macros/structures defined in
> > > > > UefiCpuPkg/Include/Register/Cpuid.h, the patch
> > moves
> > > > Cpuid.h to
> > > > > MdePkg/Include/Register/ directory.
> > > > >
> > > > > Because almost every module depends on MdePkg.dec
> > in
> > > > its INF file, the
> > > > > move almost has no impact.
> > > > >
> > > > > Signed-off-by: Ray Ni <ray.ni@intel.com>
> > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > Cc: Eric Dong <eric.dong@intel.com>
> > > > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > > > ---
> > > > > {UefiCpuPkg => MdePkg}/Include/Register/Cpuid.h
> > | 0
> > > > > 1 file changed, 0 insertions(+), 0 deletions(-)
> > > > rename {UefiCpuPkg
> > > > > => MdePkg}/Include/Register/Cpuid.h (100%)
> > > > >
> > > > > diff --git a/UefiCpuPkg/Include/Register/Cpuid.h
> > > > > b/MdePkg/Include/Register/Cpuid.h similarity
> > index
> > > > 100% rename from
> > > > > UefiCpuPkg/Include/Register/Cpuid.h
> > > > > rename to MdePkg/Include/Register/Cpuid.h
> > > > > --
> > > > > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
2019-08-01 2:26 ` Ni, Ray
@ 2019-08-01 8:15 ` Michael D Kinney
0 siblings, 0 replies; 21+ messages in thread
From: Michael D Kinney @ 2019-08-01 8:15 UTC (permalink / raw)
To: Ni, Ray, Gao, Liming, devel@edk2.groups.io, Kinney, Michael D
Cc: Dong, Eric, Laszlo Ersek
Ray,
Yes. I think it makes sense to move both.
Please include the contributor of Amd/Cpuid.h to
the reviews of that move. It would be good to see
as much feedback as possible on this change.
Thanks,
Mike
> -----Original Message-----
> From: Ni, Ray
> Sent: Wednesday, July 31, 2019 7:27 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> Gao, Liming <liming.gao@intel.com>;
> devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> Cpuid.h from UefiCpuPkg to MdePkg
>
> Mike,
> Thanks for the time for offline discussion.
> I agree with the idea to have an Intel/Cpuid.h in
> MdePkg/Include/Register/ directory.
>
> Do you prefer to move Amd/Cpuid.h to
> MdePkg/Include/Register/ in my V3 patch?
>
> Thanks,
> Ray
>
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Wednesday, July 31, 2019 12:31 AM
> > To: Gao, Liming <liming.gao@intel.com>; Ni, Ray
> <ray.ni@intel.com>;
> > devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> > Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> Cpuid.h from
> > UefiCpuPkg to MdePkg
> >
> > Liming,
> >
> > Yes. That would be the correct include statement for
> a module/lib
> > that depends on MdePkg.
> >
> > I know we have modules/libs that use the UefiCpuPkg
> and use #include
> > <Register/Cpuid.h>. If we want to provide
> compatibility with all
> > consumers of the UefiCpuPkg, the Cpuid.h file in the
> UefiCpuPkg can
> > updated to simply include <Register/Intel/Cpuid.h>
> from MdePkg. We
> > could also choose to update those consumers to use
> the new path in the
> > MdePkg.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Gao, Liming
> > > Sent: Monday, July 29, 2019 8:49 PM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> Ni, Ray
> > > <ray.ni@intel.com>; devel@edk2.groups.io
> > > Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek
> > > <lersek@redhat.com>
> > > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> Cpuid.h from
> > > UefiCpuPkg to MdePkg
> > >
> > > Mike:
> > > OK. If so, the consumer code also need to
> obviously include vendor
> > > header file, such as #include
> <Register/Intel/Cpuid.h>. Right?
> > >
> > > Thanks
> > > Liming
> > > > -----Original Message-----
> > > > From: Kinney, Michael D
> > > > Sent: Tuesday, July 30, 2019 11:42 AM
> > > > To: Gao, Liming <liming.gao@intel.com>; Ni, Ray
> > > <ray.ni@intel.com>;
> > > > devel@edk2.groups.io; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > > Cc: Dong, Eric <eric.dong@intel.com>; Laszlo
> Ersek
> > > <lersek@redhat.com>
> > > > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> > > Cpuid.h from
> > > > UefiCpuPkg to MdePkg
> > > >
> > > > Liming,
> > > >
> > > > There is an unresolved discussion on the location
> of
> > > this file in the
> > > > MdePkg. I prefer a vendor name in the path.
> > > >
> > > > MdePkg/Include/Register/Intel/Cpuid.h
> > > >
> > > > Also, after this file is moved to MdePkg, we can
> > > enter new BZs to
> > > > update all the modules that call AsmCpuid and
> > > AsmCpuidEx to use the
> > > > defines and structures from Cpuid.h.
> > > >
> > > > Mike
> > > >
> > > > > -----Original Message-----
> > > > > From: Gao, Liming
> > > > > Sent: Monday, July 29, 2019 8:29 PM
> > > > > To: Ni, Ray <ray.ni@intel.com>;
> > > devel@edk2.groups.io
> > > > > Cc: Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > > Dong, Eric
> > > > > <eric.dong@intel.com>; Laszlo Ersek
> > > <lersek@redhat.com>
> > > > > Subject: RE: [PATCH V2 5/6] MdePkg/Cpuid.h:
> Move
> > > Cpuid.h from
> > > > > UefiCpuPkg to MdePkg
> > > > >
> > > > > The change is good to me. Reviewed-by: Liming
> Gao
> > > > > <liming.gao@intel.com>
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ni, Ray
> > > > > > Sent: Wednesday, July 24, 2019 6:00 PM
> > > > > > To: devel@edk2.groups.io
> > > > > > Cc: Kinney, Michael D
> > > <michael.d.kinney@intel.com>;
> > > > > Gao, Liming
> > > > > > <liming.gao@intel.com>; Dong, Eric
> > > > > <eric.dong@intel.com>; Laszlo Ersek
> > > > > > <lersek@redhat.com>
> > > > > > Subject: [PATCH V2 5/6] MdePkg/Cpuid.h: Move
> > > Cpuid.h
> > > > > from UefiCpuPkg
> > > > > > to MdePkg
> > > > > >
> > > > > > REF:
> > > > >
> https://bugzilla.tianocore.org/show_bug.cgi?id=2008
> > > > > >
> > > > > > MdeModulePkg/DxeIpl needs to get CPUID output
> for
> > > CPU
> > > > > 5-level paging
> > > > > > capability detection.
> > > > > >
> > > > > > In order to use the macros/structures defined
> in
> > > > > > UefiCpuPkg/Include/Register/Cpuid.h, the
> patch
> > > moves
> > > > > Cpuid.h to
> > > > > > MdePkg/Include/Register/ directory.
> > > > > >
> > > > > > Because almost every module depends on
> MdePkg.dec
> > > in
> > > > > its INF file, the
> > > > > > move almost has no impact.
> > > > > >
> > > > > > Signed-off-by: Ray Ni <ray.ni@intel.com>
> > > > > > Cc: Michael D Kinney
> <michael.d.kinney@intel.com>
> > > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > > Cc: Eric Dong <eric.dong@intel.com>
> > > > > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > > > > ---
> > > > > > {UefiCpuPkg =>
> MdePkg}/Include/Register/Cpuid.h
> > > | 0
> > > > > > 1 file changed, 0 insertions(+), 0
> deletions(-)
> > > > > rename {UefiCpuPkg
> > > > > > => MdePkg}/Include/Register/Cpuid.h (100%)
> > > > > >
> > > > > > diff --git
> a/UefiCpuPkg/Include/Register/Cpuid.h
> > > > > > b/MdePkg/Include/Register/Cpuid.h similarity
> > > index
> > > > > 100% rename from
> > > > > > UefiCpuPkg/Include/Register/Cpuid.h
> > > > > > rename to MdePkg/Include/Register/Cpuid.h
> > > > > > --
> > > > > > 2.21.0.windows.1
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2019-08-01 8:15 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-24 10:00 [PATCH V2 0/6] Support 5-level paging in DXE long mode Ni, Ray
2019-07-24 10:00 ` [PATCH V2 1/6] UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled Ni, Ray
2019-07-24 10:00 ` [PATCH V2 2/6] UefiCpuPkg/CpuDxe: Remove unnecessary macros Ni, Ray
2019-07-24 10:00 ` [PATCH V2 3/6] UefiCpuPkg/CpuDxe: Support parsing 5-level page table Ni, Ray
2019-07-24 10:00 ` [PATCH V2 4/6] MdeModulePkg/DxeIpl: Introduce PCD PcdUse5LevelPageTable Ni, Ray
2019-07-24 10:00 ` [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg Ni, Ray
2019-07-30 3:29 ` Liming Gao
2019-07-30 3:42 ` Michael D Kinney
2019-07-30 3:49 ` Liming Gao
2019-07-30 16:30 ` Michael D Kinney
2019-08-01 2:26 ` Ni, Ray
2019-08-01 8:15 ` Michael D Kinney
2019-07-24 10:00 ` [PATCH V2 6/6] MdeModulePkg/DxeIpl: Create 5-level page table for long mode Ni, Ray
2019-07-24 17:23 ` [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE " Michael D Kinney
2019-07-25 0:45 ` Ni, Ray
2019-07-25 0:52 ` Michael D Kinney
2019-07-25 5:39 ` Ni, Ray
2019-07-25 15:55 ` Michael D Kinney
2019-07-25 23:42 ` Ni, Ray
2019-07-26 18:17 ` Michael D Kinney
2019-07-26 10:14 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox