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

["Ni, Ray" <ray.ni@intel.com>]

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