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

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

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