[PATCH V2 0/6] Support 5-level paging in DXE long mode

[PATCH V2 0/6] Support 5-level paging in DXE long mode

[Ni, Ray]

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

[PATCH V2 1/6] UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled

[Ni, Ray]

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

[PATCH V2 2/6] UefiCpuPkg/CpuDxe: Remove unnecessary macros

[Ni, Ray]

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

[PATCH V2 3/6] UefiCpuPkg/CpuDxe: Support parsing 5-level page table

[Ni, Ray]

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

[PATCH V2 4/6] MdeModulePkg/DxeIpl: Introduce PCD PcdUse5LevelPageTable

[Ni, Ray]

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

[PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg

[Ni, Ray]

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

Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg

[Liming Gao]

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

Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg

[Michael D Kinney]

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

Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg

[Liming Gao]

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

Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg

[Michael D Kinney]

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

Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg

[Ni, Ray]

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

Re: [PATCH V2 5/6] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg

[Michael D Kinney]

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

[PATCH V2 6/6] MdeModulePkg/DxeIpl: Create 5-level page table for long mode

[Ni, Ray]

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

Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode

[Michael D Kinney]

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
> 
> 
> 

Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode

[Ni, Ray]

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
> >
> >
> > 

Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode

[Michael D Kinney]

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
> > >
> > >
> > > 

Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode

[Ni, Ray]

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
> > > >
> > > >
> > > > 

Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode

[Michael D Kinney]

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
> > > > >
> > > > >
> > > > > 

Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode

[Ni, Ray]

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
>>>>>> 
>>>>>> 
>>>>>> 
> 

Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode

[Michael D Kinney]

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
> >>>>>>
> >>>>>>
> >>>>>> 
> >

Re: [edk2-devel] [PATCH V2 0/6] Support 5-level paging in DXE long mode

[Laszlo Ersek]

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