From: "Ard Biesheuvel" <ardb@kernel.org>
To: devel@edk2.groups.io, dun.tan@intel.com
Subject: Re: [edk2-devel] [Patch V3 0/8] Create page table by CpuPageTableLib in DxeIpl
Date: Mon, 24 Apr 2023 19:23:48 +0200 [thread overview]
Message-ID: <CAMj1kXGbb2tRQPr_38+gePnnZUdUwCGY1mANCqpRddxDBaL=Zg@mail.gmail.com> (raw)
In-Reply-To: <20230424100552.2718-1-dun.tan@intel.com>
On Mon, 24 Apr 2023 at 12:06, duntan <dun.tan@intel.com> wrote:
>
> In V3 patch set:
> 1. Add a new patch 'MdePkg: Move CpuPageTableLib defination to MdePkg' to Move CpuPageTableLib defination from UefiCpuPkg to MdePkg.
> So that MdeModulePkg doesn't need to depend on UefiCpuPkg.
As I replied to the other patch, I think adding CpuPageTableLib to
MdePkg in its current form (even only the interface) is not the right
approach here. The function protoypes and enums exposed by this
library are highly specific to a particular implementation.
There is a clear use case here, which is the DXE IPL code, and as has
been suggested in the other thread, it would be more appropriate to
define an abstraction regarding this use case, and define it as a
library class (with a NULL implementation) in MdeModulePkg. That way,
UefiCpuPkg can provide the x86 implementation based on
CpuPageTableLilb, and other architectures can do likewise.
Please refer to the patch below, which illustrates why there are other
requirements in this area:
https://edk2.groups.io/g/devel/message/101122
--
Ard.
> 2. Modify the patch 'MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib' to set GHCB page to be mapped as unencrypted for each CPU for AMD SEV feature.
>
> Dun Tan (8):
> MdePkg: Move CpuPageTableLib defination to MdePkg
> EmulatorPkg: Add CpuPageTableLib required by DxeIpl in DSC
> IntelFsp2Pkg: Add CpuPageTableLib required by DxeIpl in DSC
> MdeModulePkg: Add CpuPageTableLib required by DxeIpl in DSC
> OvmfPkg: Add CpuPageTableLib required by DxeIpl in DSC file
> MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib
> MdeModulePkg/DxeIpl: Remove duplicated code to enable NX
> MdeModulePkg/DxeIpl: Refinement to the code to set PageTable as RO
>
> EmulatorPkg/EmulatorPkg.dsc | 3 ++-
> IntelFsp2Pkg/Tools/Tests/QemuFspPkg.dsc | 3 ++-
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.h | 3 ++-
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 5 ++++-
> MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 112 ++++------------------------------------------------------------------------------------------------------------
> MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 5 +++--
> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 720 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 182 ++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> MdeModulePkg/MdeModulePkg.dsc | 3 ++-
> {UefiCpuPkg => MdePkg}/Include/Library/CpuPageTableLib.h | 0
> MdePkg/MdePkg.dec | 5 ++++-
> OvmfPkg/AmdSev/AmdSevX64.dsc | 2 +-
> OvmfPkg/Bhyve/BhyveX64.dsc | 3 ++-
> OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
> OvmfPkg/Microvm/MicrovmX64.dsc | 2 +-
> OvmfPkg/OvmfPkgIa32.dsc | 3 ++-
> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
> OvmfPkg/OvmfPkgX64.dsc | 2 +-
> OvmfPkg/OvmfXen.dsc | 2 +-
> UefiCpuPkg/UefiCpuPkg.dec | 3 ---
> 20 files changed, 211 insertions(+), 851 deletions(-)
> rename {UefiCpuPkg => MdePkg}/Include/Library/CpuPageTableLib.h (100%)
>
> --
> 2.31.1.windows.1
>
>
>
>
>
>
next prev parent reply other threads:[~2023-04-24 17:24 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-24 10:05 [Patch V3 0/8] Create page table by CpuPageTableLib in DxeIpl duntan
2023-04-24 10:05 ` [Patch V3 1/8] MdePkg: Move CpuPageTableLib defination to MdePkg duntan
2023-04-24 10:05 ` [Patch V3 2/8] EmulatorPkg: Add CpuPageTableLib required by DxeIpl in DSC duntan
2023-04-24 10:05 ` [Patch V3 3/8] IntelFsp2Pkg: " duntan
2023-04-24 10:05 ` [Patch V3 4/8] MdeModulePkg: " duntan
2023-04-24 10:05 ` [Patch V3 5/8] OvmfPkg: Add CpuPageTableLib required by DxeIpl in DSC file duntan
2023-04-24 10:05 ` [Patch V3 6/8] MdeModulePkg/DxeIpl: Create page table by CpuPageTableLib duntan
2023-04-24 10:05 ` [Patch V3 7/8] MdeModulePkg/DxeIpl: Remove duplicated code to enable NX duntan
2023-04-24 10:05 ` [Patch V3 8/8] MdeModulePkg/DxeIpl: Refinement to the code to set PageTable as RO duntan
2023-04-24 17:23 ` Ard Biesheuvel [this message]
2023-04-24 17:51 ` [edk2-devel] [Patch V3 0/8] Create page table by CpuPageTableLib in DxeIpl Michael D Kinney
2023-04-24 18:07 ` Ard Biesheuvel
2023-04-25 2:45 ` Ni, Ray
2023-04-25 17:11 ` Ard Biesheuvel
2023-04-26 6:08 ` Ni, Ray
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='CAMj1kXGbb2tRQPr_38+gePnnZUdUwCGY1mANCqpRddxDBaL=Zg@mail.gmail.com' \
--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