Re: [edk2-devel] [PATCH v1 2/7] MdeModulePkg: Invoke GetPhysicalAddressBits() and remove the duplicated code

["Pu, Yu" <yu.pu@intel.com>]

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.ni@intel.com>
Sent: Monday, March 7, 2022 11:55 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io; Pu, Yu <yu.pu@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/MdeModulePkg.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/BaseUefiCpuLib

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.pu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@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.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Yu Pu <yu.pu@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/BootScriptExecutorDxe.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/DxeServicesTableLib.inf

   UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf

   VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf

+  UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf

   #

   # Generic Modules

   #

diff --git a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf b/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
index fb149c2f0271..dd3dd2fc8c10 100644
--- a/MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.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+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
-=-=-=-=-=-=