[v2] Support 5-level paging in DXE long mode

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

Posted by Ni, Ray 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44303): https://edk2.groups.io/g/devel/message/44303
Mute This Topic: https://groups.io/mt/32582433/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Laszlo Ersek 4 years, 8 months ago

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

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44454): https://edk2.groups.io/g/devel/message/44454
Mute This Topic: https://groups.io/mt/32582433/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44324): https://edk2.groups.io/g/devel/message/44324
Mute This Topic: https://groups.io/mt/32582433/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Ni, Ray 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44347): https://edk2.groups.io/g/devel/message/44347
Mute This Topic: https://groups.io/mt/32582433/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44348): https://edk2.groups.io/g/devel/message/44348
Mute This Topic: https://groups.io/mt/32582433/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Ni, Ray 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44362): https://edk2.groups.io/g/devel/message/44362
Mute This Topic: https://groups.io/mt/32582433/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44398): https://edk2.groups.io/g/devel/message/44398
Mute This Topic: https://groups.io/mt/32582433/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Ni, Ray 4 years, 8 months ago

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

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44419): https://edk2.groups.io/g/devel/message/44419
Mute This Topic: https://groups.io/mt/32582433/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 4 years, 8 months ago

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

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44466): https://edk2.groups.io/g/devel/message/44466
Mute This Topic: https://groups.io/mt/32582433/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Ni, Ray 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44308): https://edk2.groups.io/g/devel/message/44308
Mute This Topic: https://groups.io/mt/32582439/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Liming Gao 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44583): https://edk2.groups.io/g/devel/message/44583
Mute This Topic: https://groups.io/mt/32582439/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44584): https://edk2.groups.io/g/devel/message/44584
Mute This Topic: https://groups.io/mt/32582439/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Liming Gao 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44586): https://edk2.groups.io/g/devel/message/44586
Mute This Topic: https://groups.io/mt/32582439/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44635): https://edk2.groups.io/g/devel/message/44635
Mute This Topic: https://groups.io/mt/32582439/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Ni, Ray 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44712): https://edk2.groups.io/g/devel/message/44712
Mute This Topic: https://groups.io/mt/32582439/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Michael D Kinney 4 years, 8 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44743): https://edk2.groups.io/g/devel/message/44743
Mute This Topic: https://groups.io/mt/32582439/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-