From: "gaoliming" <gaoliming@byosoft.com.cn>
To: <devel@edk2.groups.io>, <yu.pu@intel.com>,
"'Ni, Ray'" <ray.ni@intel.com>,
"'Kinney, Michael D'" <michael.d.kinney@intel.com>
Cc: "'Wang, Jian J'" <jian.j.wang@intel.com>, <gaoliming@byosoft.com.cn>
Subject: 回复: [edk2-devel] [PATCH v1 2/7] MdeModulePkg: Invoke GetPhysicalAddressBits() and remove the duplicated code
Date: Fri, 11 Mar 2022 09:02:00 +0800 [thread overview]
Message-ID: <042b01d834e3$9d64d040$d82e70c0$@byosoft.com.cn> (raw)
In-Reply-To: <BN8PR11MB35387F35C22288BB3EAE5044E80B9@BN8PR11MB3538.namprd11.prod.outlook.com>
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.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> 抄送: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <gaoliming@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.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/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.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/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+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
> -=-=-=-=-=-=
>
>
>
>
>
>
next prev parent reply other threads:[~2022-03-11 1:02 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-02 9:18 [PATCH v1 0/7] *** SUBJECT HERE *** Yu Pu
2022-03-02 9:18 ` [PATCH v1 1/7] UefiCpuPackage: Add APIs for CPU physical address mask calculation Yu Pu
2022-03-02 9:18 ` [PATCH v1 2/7] MdeModulePkg: Invoke GetPhysicalAddressBits() and remove the duplicated code Yu Pu
2022-03-07 3:54 ` [edk2-devel] " Ni, Ray
2022-03-10 8:47 ` Pu, Yu
2022-03-11 1:02 ` gaoliming [this message]
2022-03-02 9:18 ` [PATCH v1 3/7] IntelFsp2Pkg: " Yu Pu
2022-03-02 9:26 ` Chiu, Chasel
2022-03-02 9:18 ` [PATCH v1 4/7] StandaloneMmPkg: " Yu Pu
2022-03-03 9:58 ` Ard Biesheuvel
2022-03-04 7:39 ` [edk2-devel] " Ni, Ray
2022-03-02 9:18 ` [PATCH v1 5/7] OvmfPkg: " Yu Pu
2022-03-02 9:18 ` [PATCH v1 6/7] EmulatorPkg: fixed a bug caused by Invoking GetPhysicalAddressBits() Yu Pu
2022-03-02 9:18 ` [PATCH v1 7/7] UefiPayloadPkg: Invoke GetPhysicalAddressBits() and remove the duplicated code Yu Pu
2022-03-04 18:09 ` Guo Dong
[not found] ` <16D93FE5C3E39310.28436@groups.io>
2022-03-05 3:43 ` [edk2-devel] " Guo Dong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='042b01d834e3$9d64d040$d82e70c0$@byosoft.com.cn' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox