* [Patch V7 00/14] Use CpuPageTableLib to create and update smm page table @ 2023-06-27 5:23 duntan 2023-06-27 5:23 ` [Patch V7 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan 0 siblings, 1 reply; 4+ messages in thread From: duntan @ 2023-06-27 5:23 UTC (permalink / raw) To: devel In the V7 patch set: In 'OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry', remove the code to apply AddressEncMask to PageMapLevel4Entry in InternalMemEncryptSevCreateIdentityMap1G() function. Only resend the patch in OvmfPkg for review. Other patches in the patch set have been reviewed-by. Dun Tan (14): OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry MdeModulePkg: Remove other attribute protection in UnsetGuardPage UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute. UefiCpuPkg: Add DEBUG_CODE for special case when clear RP UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h UefiCpuPkg: Add GenSmmPageTable() to create smm page table UefiCpuPkg: Use GenSmmPageTable() to create Smm S3 page table UefiCpuPkg: Sort mSmmCpuSmramRanges in FindSmramInfo UefiCpuPkg: Sort mProtectionMemRange when ReadyToLock UefiCpuPkg: Refinement to smm runtime InitPaging() code UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 16 +++++++++++++++- OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 8 ++++---- 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 | 40 ++++++++++++++++++++++++++++++++++++++-- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 795 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 324 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 229 ++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 3 +-- UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 20 ++++---------------- 14 files changed, 683 insertions(+), 1016 deletions(-) -- 2.31.1.windows.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Patch V7 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry 2023-06-27 5:23 [Patch V7 00/14] Use CpuPageTableLib to create and update smm page table duntan @ 2023-06-27 5:23 ` duntan 2023-06-27 13:13 ` Lendacky, Thomas 0 siblings, 1 reply; 4+ messages in thread From: duntan @ 2023-06-27 5:23 UTC (permalink / raw) To: devel Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Tom Lendacky, Ray Ni Remove code that apply AddressEncMask to non-leaf entry when split smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask bit in page table for a specific range. In AMD SEV feature, this AddressEncMask bit in page table is used to indicate if the memory is guest private memory or shared memory. But all memory used by page table are treated as encrypted regardless of encryption bit. So remove the EncMask bit for smm non-leaf page table entry doesn't impact AMD SEV feature. If page split happens in the AddressEncMask bit clear process, there will be some new non-leaf entries with AddressEncMask applied in smm page table. When ReadyToLock, code in PiSmmCpuDxe module will use CpuPageTableLib to modify smm page table. So remove code to apply AddressEncMask for new non-leaf entries since CpuPageTableLib doesn't consume the EncMask PCD. Signed-off-by: Dun Tan <dun.tan@intel.com> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Reviewed-by: Ray Ni <ray.ni@intel.com> --- OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c index cf2441b551..372fc03fde 100644 --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c @@ -233,7 +233,7 @@ Split2MPageTo4K ( // Fill in 2M page entry. // *PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 | - IA32_PG_P | IA32_PG_RW | AddressEncMask); + IA32_PG_P | IA32_PG_RW); } /** @@ -352,7 +352,7 @@ SetPageTablePoolReadOnly ( PhysicalAddress += LevelSize[Level - 1]; } - PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask | + PageTable[Index] = (UINT64)(UINTN)NewPageTable | IA32_PG_P | IA32_PG_RW; PageTable = NewPageTable; } @@ -440,7 +440,7 @@ Split1GPageTo2M ( // Fill in 1G page entry. // *PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry | - IA32_PG_P | IA32_PG_RW | AddressEncMask); + IA32_PG_P | IA32_PG_RW); PhysicalAddress2M = PhysicalAddress; for (IndexOfPageDirectoryEntries = 0; @@ -616,7 +616,7 @@ InternalMemEncryptSevCreateIdentityMap1G ( } SetMem (NewPageTable, EFI_PAGE_SIZE, 0); - PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable | AddressEncMask; + PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable; PageMapLevel4Entry->Bits.MustBeZero = 0; PageMapLevel4Entry->Bits.ReadWrite = 1; PageMapLevel4Entry->Bits.Present = 1; -- 2.31.1.windows.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Patch V7 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry 2023-06-27 5:23 ` [Patch V7 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan @ 2023-06-27 13:13 ` Lendacky, Thomas 2023-06-29 10:09 ` [edk2-devel] " duntan 0 siblings, 1 reply; 4+ messages in thread From: Lendacky, Thomas @ 2023-06-27 13:13 UTC (permalink / raw) To: Dun Tan, devel Cc: Ard Biesheuvel, Jiewen Yao, Jordan Justen, Gerd Hoffmann, Ray Ni On 6/27/23 00:23, Dun Tan wrote: > Remove code that apply AddressEncMask to non-leaf entry when split s/apply/applies the/ s/entry/entries/ s/split/splitting/ > smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it s/smm page table by/SMM page table entries in/ s/In FvbServicesSmm driver, it/The FvbServicesSmm driver/ > calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask s/clear/clear the/ > bit in page table for a specific range. In AMD SEV feature, this > AddressEncMask bit in page table is used to indicate if the memory > is guest private memory or shared memory. But all memory used by > page table are treated as encrypted regardless of encryption bit. But all memory accessed by the hardware page table walker is treated as encrypted, regardless of whether the encryption bit is present. > So remove the EncMask bit for smm non-leaf page table entry > doesn't impact AMD SEV feature. > If page split happens in the AddressEncMask bit clear process, > there will be some new non-leaf entries with AddressEncMask > applied in smm page table. When ReadyToLock, code in PiSmmCpuDxe > module will use CpuPageTableLib to modify smm page table. So > remove code to apply AddressEncMask for new non-leaf entries > since CpuPageTableLib doesn't consume the EncMask PCD. This last paragraph is a bit confusing to read, please rewrite it so it is easier to understand. > > Signed-off-by: Dun Tan <dun.tan@intel.com> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Reviewed-by: Ray Ni <ray.ni@intel.com> I think it would be best to include comments in the code around the areas being changed explaining why the the encryption mask is not being set for non-leaf entries because of the way CpuPageTableLib works. With comments added: Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > index cf2441b551..372fc03fde 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > @@ -233,7 +233,7 @@ Split2MPageTo4K ( > // Fill in 2M page entry. > // > *PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 | > - IA32_PG_P | IA32_PG_RW | AddressEncMask); > + IA32_PG_P | IA32_PG_RW); > } > > /** > @@ -352,7 +352,7 @@ SetPageTablePoolReadOnly ( > PhysicalAddress += LevelSize[Level - 1]; > } > > - PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask | > + PageTable[Index] = (UINT64)(UINTN)NewPageTable | > IA32_PG_P | IA32_PG_RW; > PageTable = NewPageTable; > } > @@ -440,7 +440,7 @@ Split1GPageTo2M ( > // Fill in 1G page entry. > // > *PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry | > - IA32_PG_P | IA32_PG_RW | AddressEncMask); > + IA32_PG_P | IA32_PG_RW); > > PhysicalAddress2M = PhysicalAddress; > for (IndexOfPageDirectoryEntries = 0; > @@ -616,7 +616,7 @@ InternalMemEncryptSevCreateIdentityMap1G ( > } > > SetMem (NewPageTable, EFI_PAGE_SIZE, 0); > - PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable | AddressEncMask; > + PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable; > PageMapLevel4Entry->Bits.MustBeZero = 0; > PageMapLevel4Entry->Bits.ReadWrite = 1; > PageMapLevel4Entry->Bits.Present = 1; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [Patch V7 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry 2023-06-27 13:13 ` Lendacky, Thomas @ 2023-06-29 10:09 ` duntan 0 siblings, 0 replies; 4+ messages in thread From: duntan @ 2023-06-29 10:09 UTC (permalink / raw) To: devel@edk2.groups.io, thomas.lendacky@amd.com Cc: Ard Biesheuvel, Yao, Jiewen, Justen, Jordan L, Gerd Hoffmann, Ni, Ray Tom, Thanks for the comments. In V8 patch, I've refined the commit message and added comments in the code around the areas being changed to explain this code change. Thanks, Dun -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Lendacky, Thomas via groups.io Sent: Tuesday, June 27, 2023 9:13 PM To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Ni, Ray <ray.ni@intel.com> Subject: Re: [edk2-devel] [Patch V7 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry On 6/27/23 00:23, Dun Tan wrote: > Remove code that apply AddressEncMask to non-leaf entry when split s/apply/applies the/ s/entry/entries/ s/split/splitting/ > smm page table by MemEncryptSevLib. In FvbServicesSmm driver, it s/smm page table by/SMM page table entries in/ s/In FvbServicesSmm driver, it/The FvbServicesSmm driver/ > calls MemEncryptSevClearMmioPageEncMask to clear AddressEncMask s/clear/clear the/ > bit in page table for a specific range. In AMD SEV feature, this > AddressEncMask bit in page table is used to indicate if the memory is > guest private memory or shared memory. But all memory used by page > table are treated as encrypted regardless of encryption bit. But all memory accessed by the hardware page table walker is treated as encrypted, regardless of whether the encryption bit is present. > So remove the EncMask bit for smm non-leaf page table entry doesn't > impact AMD SEV feature. > If page split happens in the AddressEncMask bit clear process, there > will be some new non-leaf entries with AddressEncMask applied in smm > page table. When ReadyToLock, code in PiSmmCpuDxe module will use > CpuPageTableLib to modify smm page table. So remove code to apply > AddressEncMask for new non-leaf entries since CpuPageTableLib doesn't > consume the EncMask PCD. This last paragraph is a bit confusing to read, please rewrite it so it is easier to understand. > > Signed-off-by: Dun Tan <dun.tan@intel.com> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Reviewed-by: Ray Ni <ray.ni@intel.com> I think it would be best to include comments in the code around the areas being changed explaining why the the encryption mask is not being set for non-leaf entries because of the way CpuPageTableLib works. With comments added: Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git > a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > index cf2441b551..372fc03fde 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > @@ -233,7 +233,7 @@ Split2MPageTo4K ( > // Fill in 2M page entry. > // > *PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 | > - IA32_PG_P | IA32_PG_RW | AddressEncMask); > + IA32_PG_P | IA32_PG_RW); > } > > /** > @@ -352,7 +352,7 @@ SetPageTablePoolReadOnly ( > PhysicalAddress += LevelSize[Level - 1]; > } > > - PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask | > + PageTable[Index] = (UINT64)(UINTN)NewPageTable | > IA32_PG_P | IA32_PG_RW; > PageTable = NewPageTable; > } > @@ -440,7 +440,7 @@ Split1GPageTo2M ( > // Fill in 1G page entry. > // > *PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry | > - IA32_PG_P | IA32_PG_RW | AddressEncMask); > + IA32_PG_P | IA32_PG_RW); > > PhysicalAddress2M = PhysicalAddress; > for (IndexOfPageDirectoryEntries = 0; @@ -616,7 +616,7 @@ > InternalMemEncryptSevCreateIdentityMap1G ( > } > > SetMem (NewPageTable, EFI_PAGE_SIZE, 0); > - PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable | AddressEncMask; > + PageMapLevel4Entry->Uint64 = (UINT64)(UINTN)NewPageTable; > PageMapLevel4Entry->Bits.MustBeZero = 0; > PageMapLevel4Entry->Bits.ReadWrite = 1; > PageMapLevel4Entry->Bits.Present = 1; ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-29 10:09 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-27 5:23 [Patch V7 00/14] Use CpuPageTableLib to create and update smm page table duntan 2023-06-27 5:23 ` [Patch V7 01/14] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan 2023-06-27 13:13 ` Lendacky, Thomas 2023-06-29 10:09 ` [edk2-devel] " duntan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox