On 4/13/23 04:14, Tan, Dun wrote:
Hi Tom,
Thank you for your help with testing.
For the build failure, it's because that the CpuPageTableLib instance is added
into OvmfPkg DSC in the last pacth ' OvmfPkg: Add CpuPageTableLib required by
PiSmmCpuDxe'. I have moved this patch to the head of the patch set.
For the boot failure, I think it's because that the encrypt mask was not
applied to the memory used by page table in page table non-leaf entry.
Initially I thought the encrypt mask would only be applied to the leaf entry in
AMD SEV feature. So I treated the encryption process as non 1:1 mapping, which
only applies the encrypt mask to leaf entry. I'm also curious why the DxeIpl
patch set works good. All the page table non-leaf entries are also not
encrypted in the DxeIpl page table related patch set.
Right, and that works for SEV. All non-leaf pagetable entries are treated
as encrypted regardless of the encryption bit. Since the tables were built
being mapped encrypted, the pagetable walk works when the non-leaf entries
don't have the encryption bit set. In this case, though, the encryption
bit is present in the non-leaf entry and that is the reason why there are
issues.
Here is some debug after setting PagingEntry at line 436 of
UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c:
*** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF81000
*** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF80000
*** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF83000
*** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF81000
*** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF80000
*** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF83000
*** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF81000
*** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 3FF80000
*** DEBUG: PageTableLibMapInLevel:437 - PagingEntry = 800003FC01000
!!!! X64 Exception Type - 0D(#GP - General Protection) CPU Apic ID - 00000000
!!!!
0x800003FC01000 isn't mapped and so it fails - I'm not exactly sure how
the #PF turns into a #GP, though, maybe because the virtual address isn't
canonical that point.
Thanks,
Tom
I'll added another patch in my code branch to fix this issue later. In the new commit,
from the perspective of CpuPageTableLib, the whole memory can be divided into 3
categories: memory used by page table, guest private memory and guest shared memory.
CpuPageTableLib will always apply the encrypt mask to memory used by page table, which
means all the non-leaf page table entries are encrypted. For guest private memory, this
case can be treated as non-1:1 mapping. We can apply the encrypt mask by setting the
input parameter of PageTableMap() API like " Attribute.Uint64 = LinearAddress |
AddressEncMask". For guest shared memory, this case can be treated as normal 1:1
mapping. I'll let you know once the new patch is ready.
Thanks,
Dun
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Lendacky, Thomas
via groups.io
Sent: Thursday, April 13, 2023 3:26 AM
To: devel@edk2.groups.io; Tan, Dun <dun....@intel.com>
Subject: Re: [edk2-devel] [Patch V2 0/8] Use CpuPageTableLib to create and
update smm page table
On 4/12/23 05:17, duntan via groups.io wrote:
Hi Tom,
This patch set is to change PiSmmCpuDxeSmm code to use CpuPageTableLib to
create and update SMM page table. The Pcd PcdPteMemoryEncryptionAddressOrMask
is also used in PiSmmCpuDxeSmm code and the whole range covered by page table
is mapped encrypted, which is different from the situation in DxeIpl module.
So could you also help do a test to make sure the AMD SEV feature still works
good in SMM with this patch set?
Here is the code branch in my fork repo:
https://github.com/td36/edk2/commits/SmmPageTable_V2
Hi Dun,
I tested at the final commit of the branch and encountered a #GP with an SEV
guest. It looks like the CpuPageTableLibrary doesn't take the encryption bit
into account. For example:
Line 436 of UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
PagingEntry = (IA32_PAGING_ENTRY *)(UINTN)IA32_PNLE_PAGE_TABLE_BASE_ADDRESS
(&ParentPagingEntry->Pnle);
This will get an address with the encryption bit set and then try to reference
it. When I clear the encryption bit, the code proceeds a bit further, but then
encounters a #GP in a different location.
So it appears that the CpuPageTableLibrary doesn't deal with the encryption bit
properly.
Also, going through a build/test of each individual patch had mixed results.
- With the second patch in the series applied, I get a build error:
/root/kernels/ovmf-dun-build-X64/OvmfPkg/OvmfPkgX64.dsc(...): error 4000:
Instance of library class [CpuPageTableLib] is not found
in
[/root/kernels/ovmf-dun-build-X64/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf]
[X64]
consumed by module
[/root/kernels/ovmf-dun-build-X64/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf]
that isn't resolved until the final patch.
Thanks,
Tom
Thanks,
Dun
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
Sent: Wednesday, April 12, 2023 4:54 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [Patch V2 0/8] Use CpuPageTableLib to create and
update smm page table
In V2 patch set:
1.In 'Refinement to code about updating smm page table', use QuickSort() in
BaseLib instead or PerformQuickSort() in BaseSortLib.
2.Remove the patch to add BaseSortLib in DSC file.
3.Add a new patch to add CpuPageTableLib in UefiCpuPkg.dsc.
4.Add a temp patch to add CpuPageTableLib in OvmfPkg dsc files for
test(A previous patch I sent before '[Patch V2 4/8] OvmfPkg: Add
CpuPageTableLib required by DxeIpl in DSC file' contains all the
changes in this patch)
Dun Tan (8):
OvmfPkg: Add CpuPageTableLib required by PiSmmCpuDxe
UefiPayloadPkg: Add CpuPageTableLib required by PiSmmCpuDxe
UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute.
UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX
UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h
UefiCpuPkg: Refinement to current smm page table generation code
UefiCpuPkg: Refinement to code about updating smm page table
UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function
OvmfPkg/CloudHv/CloudHvX64.dsc | 2 +-
OvmfPkg/OvmfPkgIa32.dsc | 3 ++-
OvmfPkg/OvmfPkgIa32X64.dsc | 2 +-
OvmfPkg/OvmfPkgX64.dsc | 2 +-
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 5 +++--
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 3 +--
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c | 2 +-
UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 132
------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 8 ++++++--
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 97
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 +
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 629
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 348
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 229
++++++++++++++++++++++++++++++-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 3 +--
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 19 ++-----------------
UefiPayloadPkg/UefiPayloadPkg.dsc | 2 +-
17 files changed, 510 insertions(+), 977 deletions(-)
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102957): https://edk2.groups.io/g/devel/message/102957
Mute This Topic: https://groups.io/mt/98215421/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
