Yu and Ray:
  I review UefiCpuLib. I think they are generic enough (X86 arch) to be
placed in MdePkg.

Mike:
  What's your comments for this proposal?

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Pu, Yu
> 发送时间: 2022年3月10日 16:47
> 收件人: Ni, Ray <ray...@intel.com>; Kinney, Michael D
> <michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>
> 抄送: Wang, Jian J <jian.j.w...@intel.com>; Gao, Liming
> <gaolim...@byosoft.com.cn>; devel@edk2.groups.io
> 主题: Re: [edk2-devel] [PATCH v1 2/7] MdeModulePkg: Invoke
> GetPhysicalAddressBits() and remove the duplicated code
> 
> Hi, Mike, Liming,
> 
> This patch makes MdeModulePkg depend on UefiCpuPkg.
> Ray and me would like to move the UefiCpuLib from UefiCpuPkg to MdePkg,
> do you agree with it ?
> 
> Thanks,
> Yu
> 
> -----Original Message-----
> From: Ni, Ray <ray...@intel.com>
> Sent: Monday, March 7, 2022 11:55 AM
> To: Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming
> <gaolim...@byosoft.com.cn>
> Cc: Wang, Jian J <jian.j.w...@intel.com>; Gao, Liming
> <gaolim...@byosoft.com.cn>; devel@edk2.groups.io; Pu, Yu
> <yu...@intel.com>
> Subject: RE: [edk2-devel] [PATCH v1 2/7] MdeModulePkg: Invoke
> GetPhysicalAddressBits() and remove the duplicated code
> 
> Mike, Liming,
> This patch makes MdeModulePkg depend on UefiCpuPkg.
> But according to
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/MdeModul
> ePkg.ci.yaml#L49, such dependency is not allowed.
> 
> Do you agree to move the UefiCpuLib from UefiCpuPkg to MdePkg? The
> library header and instance are in following paths:
> 
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Include/Library
> /UefiCpuLib.h
> 
> https://github.com/tianocore/edk2/tree/master/UefiCpuPkg/Library/BaseUe
> fiCpuLib
> 
> Thanks,
> Ray
> 
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yu Pu
> Sent: Wednesday, March 2, 2022 5:19 PM
> To: devel@edk2.groups.io
> Cc: Pu, Yu <yu...@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; Gao,
> Liming <gaolim...@byosoft.com.cn>
> Subject: [edk2-devel] [PATCH v1 2/7] MdeModulePkg: Invoke
> GetPhysicalAddressBits() and remove the duplicated code
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3394
> 
> Invoke GetPhysicalAddressBits() defined in UefiCpuPkg for CPU physical
> address mask calculation and remove the duplicated code in MdeModulePkg.
> 
> Cc: Jian J Wang <jian.j.w...@intel.com>
> Cc: Liming Gao <gaolim...@byosoft.com.cn>
> 
> Signed-off-by: Yu Pu <yu...@intel.com>
> ---
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> |  9 ++-------
>  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
> | 14 ++------------
>  MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> | 14 ++------------
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> |  2 ++
>  MdeModulePkg/MdeModulePkg.dsc
> |  1 +
> 
> MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDx
> e.inf |  2 ++
>  MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf
> |  2 ++
>  7 files changed, 13 insertions(+), 31 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> index 0700f310b203..78e91e6e9024 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> @@ -22,6 +22,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
>  **/
> 
> 
> 
> +#include <Library/UefiCpuLib.h>
> 
>  #include <Register/Intel/Cpuid.h>
> 
>  #include "DxeIpl.h"
> 
>  #include "VirtualMemory.h"
> 
> @@ -733,13 +734,7 @@ CreateIdentityMappingPageTables (
>    if (Hob != NULL) {
> 
>      PhysicalAddressBits = ((EFI_HOB_CPU *)Hob)->SizeOfMemorySpace;
> 
>    } else {
> 
> -    AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
> 
> -    if (RegEax >= 0x80000008) {
> 
> -      AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);
> 
> -      PhysicalAddressBits = (UINT8)RegEax;
> 
> -    } else {
> 
> -      PhysicalAddressBits = 36;
> 
> -    }
> 
> +    PhysicalAddressBits = GetPhysicalAddressBits(NULL, NULL);
> 
>    }
> 
> 
> 
>    Page5LevelSupport = FALSE;
> 
> diff --git
> a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
> b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
> index 6b44f50bac70..367bf8cdd1e6 100644
> ---
> a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
> +++
> b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.
> +++ c
> @@ -10,6 +10,7 @@ Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> +#include <Library/UefiCpuLib.h>
> 
>  #include "ScriptExecute.h"
> 
> 
> 
>  //
> 
> @@ -51,20 +52,9 @@ HookPageFaultHandler (
>    IN IA32_IDT_GATE_DESCRIPTOR  *IdtEntry
> 
>    )
> 
>  {
> 
> -  UINT32  RegEax;
> 
> -  UINT8   PhysicalAddressBits;
> 
>    UINTN   PageFaultHandlerHookAddress;
> 
> 
> 
> -  AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
> 
> -  if (RegEax >= 0x80000008) {
> 
> -    AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);
> 
> -    PhysicalAddressBits = (UINT8)RegEax;
> 
> -  } else {
> 
> -    PhysicalAddressBits = 36;
> 
> -  }
> 
> -
> 
> -  mPhyMask  = LShiftU64 (1, PhysicalAddressBits) - 1;
> 
> -  mPhyMask &= (1ull << 48) - SIZE_4KB;
> 
> +  GetPhysicalAddressBits(NULL, &mPhyMask);
> 
> 
> 
>    //
> 
>    // Set Page Fault entry to catch >4G access
> 
> diff --git a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> index 05941f9f8d56..06d6129c5e6d 100644
> --- a/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> +++ b/MdeModulePkg/Universal/CapsulePei/X64/X64Entry.c
> @@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/BaseMemoryLib.h>
> 
>  #include <Library/CpuExceptionHandlerLib.h>
> 
>  #include <Library/DebugAgentLib.h>
> 
> +#include <Library/UefiCpuLib.h>
> 
>  #include "CommonHeader.h"
> 
> 
> 
>  #define EXCEPTION_VECTOR_NUMBER  0x22
> 
> @@ -61,20 +62,9 @@ HookPageFaultHandler (
>    IN OUT PAGE_FAULT_CONTEXT        *PageFaultContext
> 
>    )
> 
>  {
> 
> -  UINT32  RegEax;
> 
> -  UINT8   PhysicalAddressBits;
> 
>    UINTN   PageFaultHandlerHookAddress;
> 
> 
> 
> -  AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
> 
> -  if (RegEax >= 0x80000008) {
> 
> -    AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);
> 
> -    PhysicalAddressBits = (UINT8)RegEax;
> 
> -  } else {
> 
> -    PhysicalAddressBits = 36;
> 
> -  }
> 
> -
> 
> -  PageFaultContext->PhyMask  = LShiftU64 (1, PhysicalAddressBits) - 1;
> 
> -  PageFaultContext->PhyMask &= (1ull << 48) - SIZE_4KB;
> 
> +  GetPhysicalAddressBits(NULL, &(PageFaultContext->PhyMask));
> 
> 
> 
>    //
> 
>    // Set Page Fault entry to catch >4G access
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index 19b8a4c8aefa..45808bcdcd6c 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -55,6 +55,7 @@
> 
> 
>  [Packages]
> 
>    MdePkg/MdePkg.dec
> 
> +  UefiCpuPkg/UefiCpuPkg.dec
> 
>    MdeModulePkg/MdeModulePkg.dec
> 
> 
> 
>  [Packages.ARM, Packages.AARCH64]
> 
> @@ -75,6 +76,7 @@
>    DebugAgentLib
> 
>    PeiServicesTablePointerLib
> 
>    PerformanceLib
> 
> +  UefiCpuLib
> 
> 
> 
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> 
>    ArmMmuLib
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc index b1d83461865e..da6213c02da0
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -62,6 +62,7 @@
> 
> DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableL
> ib.inf
> 
> 
> UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBoot
> ManagerLib.inf
> 
> 
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib
> .inf
> 
> +  UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> 
>    #
> 
>    # Generic Modules
> 
>    #
> 
> diff --git
> a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutor
> Dxe.inf
> b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutor
> Dxe.inf
> index fb149c2f0271..dd3dd2fc8c10 100644
> ---
> a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutor
> Dxe.inf
> +++
> b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecut
> +++ orDxe.inf
> @@ -41,6 +41,7 @@
> 
> 
>  [Packages]
> 
>    MdePkg/MdePkg.dec
> 
> +  UefiCpuPkg/UefiCpuPkg.dec
> 
>    MdeModulePkg/MdeModulePkg.dec
> 
> 
> 
>  [LibraryClasses]
> 
> @@ -54,6 +55,7 @@
>    UefiBootServicesTableLib
> 
>    CacheMaintenanceLib
> 
>    UefiLib
> 
> +  UefiCpuLib
> 
>    DebugAgentLib
> 
>    LockBoxLib
> 
>    CpuExceptionHandlerLib
> 
> diff --git a/MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf
> b/MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf
> index 35d2535a5b48..75813b1e5481 100644
> --- a/MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf
> +++ b/MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf
> @@ -38,12 +38,14 @@
>  [Packages]
> 
>    MdePkg/MdePkg.dec
> 
>    MdeModulePkg/MdeModulePkg.dec
> 
> +  UefiCpuPkg/UefiCpuPkg.dec
> 
> 
> 
>  [LibraryClasses]
> 
>    BaseLib
> 
>    DebugLib
> 
>    CpuExceptionHandlerLib
> 
>    DebugAgentLib
> 
> +  UefiCpuLib
> 
> 
> 
>  [Depex]
> 
>    FALSE
> 
> --
> 2.30.0.windows.2
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#87235): https://edk2.groups.io/g/devel/message/87235
> Mute This Topic: https://groups.io/mt/89503325/1712937
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray...@intel.com]
> -=-=-=-=-=-=
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#87436): https://edk2.groups.io/g/devel/message/87436
Mute This Topic: https://groups.io/mt/89701429/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to