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

["Michael D Kinney" <michael.d.kinney@intel.com>]

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