Thank you a lot! I misunderstood the condition for remapping GHCB page in
previous code. I'll fix the issue in next version patch.
Thanks,
Dun
-----Original Message-----
From: Tom Lendacky <thomas.lenda...@amd.com>
Sent: Tuesday, April 4, 2023 1:14 AM
To: Tan, Dun <dun....@intel.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray...@intel.com>
Subject: Re: [edk2-devel] [Patch V2 0/8] Create page table by CpuPageTableLib
in DxeIpl
On 4/3/23 09:24, Tom Lendacky wrote:
> On 3/31/23 09:35, Tom Lendacky wrote:
>> On 3/31/23 04:41, Tan, Dun wrote:
>>> Hi Tom,
>>>
>>> Reccentlly I sent this patch set to change DxeIpl code to use
>>> CpuPageTableLib to create page table. I have done some test on Intel
>>> CPU to make sure that the page table created by DxeIpl before the
>>> change is the same as the page table created by DxeIpl after the
>>> change. But there was a remaining case that I didn't cover. The case
>>> is that PcdPteMemoryEncryptionAddressOrMask, PcdGhcbBase and
>>> PcdGhcbSize are not zero(when memory encryption is enabled on AMD
>>> processors supporting the SEV feature).
>>> So could you please help do a test on AMD processor to make sure
>>> that the SEV feature still works good with this pacth set?
>>
>> Yes, I can test it.
>
> This is breaking the SEV-ES and SEV-SNP boots. I'll see if I can
> figure out what or where the breakage is, but this patchset can't be merged
> as is.
The following change to the patch series allows SEV-ES and SEV-SNP guests to
boot.
Thanks,
Tom
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index a9edf4de32..a3f16c7cf9 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -416,6 +416,7 @@ CreateIdentityMappingPageTables (
IA32_MAP_ATTRIBUTE MapAttribute;
IA32_MAP_ATTRIBUTE MapMask;
EFI_PHYSICAL_ADDRESS GhcbBase4K;
+ EFI_PHYSICAL_ADDRESS GhcbBaseEnd;
//
// Make sure AddressEncMask is contained to smallest supported address
field @@ -504,15 +505,21 @@ CreateIdentityMappingPageTables (
//
// The GHCB range consists of two pages per CPU, the GHCB and a
// per-CPU variable page. The GHCB page needs to be mapped as an
- // unencrypted page while the per-CPU variable page needs to be
- // mapped encrypted. These pages alternate in assignment.
+ // unencrypted page while the per-CPU variable page needs to remain
+ // mapped as an encrypted page.
+ //
+ // Loop through the GHCB range, remapping the GHCB page unencrypted
+ // and skipping over the per-CPU variable page.
//
ASSERT (Is64BitPageTable == TRUE);
- GhcbBase4K = ALIGN_VALUE (GhcbBase, SIZE_4KB);
- MapAttribute.Uint64 = GhcbBase4K;
- MapMask.Uint64 = 0;
- MapMask.Bits.PageTableBaseAddressLow = 1;
- CreateOrUpdatePageTable (&PageTable, PagingMode, GhcbBase4K, SIZE_4KB,
&MapAttribute, &MapMask);
+ GhcbBase4K = ALIGN_VALUE (GhcbBase, SIZE_4KB);
+ GhcbBaseEnd = ALIGN_VALUE (GhcbBase + GhcbSize, SIZE_4KB);
+ for (; GhcbBase4K < GhcbBaseEnd; GhcbBase4K += (SIZE_4KB * 2)) {
+ MapAttribute.Uint64 = GhcbBase4K;
+ MapMask.Uint64 = 0;
+ MapMask.Bits.PageTableBaseAddressLow = 1;
+ CreateOrUpdatePageTable (&PageTable, PagingMode, GhcbBase4K, SIZE_4KB,
&MapAttribute, &MapMask);
+ }
}
if (PcdGetBool (PcdSetNxForStack)) {
>
> Thanks,
> Tom
>
>>
>> Thanks,
>> Tom
>>
>>>
>>> Thanks,
>>> Dun
>>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>>> duntan
>>> Sent: Friday, March 31, 2023 5:34 PM
>>> To: devel@edk2.groups.io
>>> Subject: [edk2-devel] [Patch V2 0/8] Create page table by
>>> CpuPageTableLib in DxeIpl
>>>
>>> In this V2 patch set:
>>> 1.Remove the unneeded patch for ArmVirtPkg 2.In this patch 'Create
>>> page table by CpuPageTableLib', change the input parameter name from
>>> Is32BitPageTable to Is64BitPageTable and add a line of
>>> "MapAttribute.Bits.Present = 0" before set a range to non-present.
>>> 3.In this patch 'Refinement to the code to set PageTable as RO', add
>>> a line of "MapAttribute.Bits.ReadWrite = 0" before set a range to ReadOnly.
>>>
>>> Dun Tan (8):
>>> 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: Add UefiCpuPkg.dec to pass DependencyCheck
>>> 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 | 4 +++-
>>> MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 112
>>> ++++----------------------------------------------------------------
>>> ++++--------------------------------------------
>>> MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 5 +++--
>>> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 711
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>>> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 182
>>> ++++++++++----------------------------------------------------------
>>> ++++++++++----------------------------------------------------------
>>> ++++++++++--------------------------------------------------------
>>> MdeModulePkg/MdeModulePkg.ci.yaml | 5 +++--
>>> MdeModulePkg/MdeModulePkg.dsc | 3 ++-
>>> 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 +-
>>> 18 files changed, 200 insertions(+), 849 deletions(-)
>>>
>>> --
>>> 2.31.1.windows.1
>>>
>>>
>>>
>>>
>>>
>>>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102452): https://edk2.groups.io/g/devel/message/102452
Mute This Topic: https://groups.io/mt/97969850/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
