In the V8 patch set:
In 'OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry', I refined the commit message and added comments in the code around the areas being changed to explain this code change.

Only resend the changed patch in OvmfPkg. The patch set has 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 |  23 +++++++++++++++++++----
 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             | 793 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                         | 322 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c                        | 229 ++++++++++++++++++++++++++++++++-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c                   |   3 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c                 |  20 ++++----------------
 14 files changed, 696 insertions(+), 1014 deletions(-)

-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106503): https://edk2.groups.io/g/devel/message/106503
Mute This Topic: https://groups.io/mt/99847923/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Thu, 29 Jun 2023 at 10:09, duntan <dun.tan@intel.com> wrote:
>
> In the V8 patch set:
> In 'OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry', I refined the commit message and added comments in the code around the areas being changed to explain this code change.
>
> Only resend the changed patch in OvmfPkg. The patch set has 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.

This patch breaks SMM on IA32.

!!!! IA32 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID - 00000000 !!!!
ExceptionData - 00000008  I:0 R:1 U:0 W:0 P:0 PK:0 SS:0 SGX:0
EIP  - 07FF97A6, CS  - 00000008, EFLAGS - 00000046
EAX  - 07FF2400, ECX - 07FC5140, EDX - 06AD7120, EBX - FFFFFFFF
ESP  - 07FCCDB4, EBP - 07FCCF4C, ESI - 00000000, EDI - 00000000
DS   - 00000020, ES  - 00000020, FS  - 00000020, GS  - 00000020, SS - 00000020
CR0  - 8001003B, CR2 - 06AD713C, CR3 - 07FA5000, CR4 - 00000668
DR0  - 00000000, DR1 - 00000000, DR2 - 00000000, DR3 - 00000000
DR6  - FFFF0FF0, DR7 - 00000400
GDTR - 07FC3000 0000004F, IDTR - 07FC6000 000000FF
LDTR - 00000000, TR - 00000040
FXSAVE_STATE - 07FC7D60
qemu: terminating on signal 2

This appears to be a result from the following code in
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c:SmmInitPageTable()

@@ -31,7 +31,7 @@ SmmInitPageTable (
   InitializeSpinLock (mPFLock);

   mPhysicalAddressBits = 32;
   mPagingMode          = PagingPae;

which seems to be the wrong paging mode. However, 'Paging32bit' is not
actually supported by the library so changing it results in an
ASSERT():

Patch page table start ...

ASSERT_RETURN_ERROR (Status = Unsupported)
ASSERT [PiSmmCpuDxeSmm]
/home/ardb/build/edk2/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c(537):
!(((INTN)(RETURN_STATUS)(Status)) < 0)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108937): https://edk2.groups.io/g/devel/message/108937
Mute This Topic: https://groups.io/mt/99847923/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Hi Ard,

Could you send me your build and boot command? 

I think the paging mode for IA32 smm should be PagingPae instead of 'Paging32bit'. Also in previous code logic before my patch PagingPae is created for IA32 smm.

Thanks,
Dun

-----Original Message-----
From: Ard Biesheuvel <ardb@kernel.org> 
Sent: Thursday, September 21, 2023 5:06 PM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [Patch V8 00/14] Subject: [Patch V8 00/14] Use CpuPageTableLib to create and update smm page table

On Thu, 29 Jun 2023 at 10:09, duntan <dun.tan@intel.com> wrote:
>
> In the V8 patch set:
> In 'OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry', I refined the commit message and added comments in the code around the areas being changed to explain this code change.
>
> Only resend the changed patch in OvmfPkg. The patch set has 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.

This patch breaks SMM on IA32.

!!!! IA32 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID - 00000000 !!!!
ExceptionData - 00000008  I:0 R:1 U:0 W:0 P:0 PK:0 SS:0 SGX:0 EIP  - 07FF97A6, CS  - 00000008, EFLAGS - 00000046 EAX  - 07FF2400, ECX - 07FC5140, EDX - 06AD7120, EBX - FFFFFFFF ESP  - 07FCCDB4, EBP - 07FCCF4C, ESI - 00000000, EDI - 00000000
DS   - 00000020, ES  - 00000020, FS  - 00000020, GS  - 00000020, SS - 00000020
CR0  - 8001003B, CR2 - 06AD713C, CR3 - 07FA5000, CR4 - 00000668
DR0  - 00000000, DR1 - 00000000, DR2 - 00000000, DR3 - 00000000
DR6  - FFFF0FF0, DR7 - 00000400
GDTR - 07FC3000 0000004F, IDTR - 07FC6000 000000FF LDTR - 00000000, TR - 00000040 FXSAVE_STATE - 07FC7D60
qemu: terminating on signal 2

This appears to be a result from the following code in
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c:SmmInitPageTable()

@@ -31,7 +31,7 @@ SmmInitPageTable (
   InitializeSpinLock (mPFLock);

   mPhysicalAddressBits = 32;
   mPagingMode          = PagingPae;

which seems to be the wrong paging mode. However, 'Paging32bit' is not actually supported by the library so changing it results in an
ASSERT():

Patch page table start ...

ASSERT_RETURN_ERROR (Status = Unsupported) ASSERT [PiSmmCpuDxeSmm]
/home/ardb/build/edk2/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c(537):
!(((INTN)(RETURN_STATUS)(Status)) < 0)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108938): https://edk2.groups.io/g/devel/message/108938
Mute This Topic: https://groups.io/mt/99847923/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On Thu, 21 Sept 2023 at 10:10, Tan, Dun <dun.tan@intel.com> wrote:
>
> Hi Ard,
>
> Could you send me your build and boot command?
>
> I think the paging mode for IA32 smm should be PagingPae instead of 'Paging32bit'. Also in previous code logic before my patch PagingPae is created for IA32 smm.
>


Build like this

build -p OvmfPkg/OvmfPkgIa32.dsc -b DEBUG -a IA32 -t GCC5 \
 -D SMM_REQUIRE -D SECURE_BOOT_ENABLE -D DEBUG_ON_SERIAL_PORT

and boot like this

qemu-system-x86_64 -M q35,smm=on -serial stdio \
-drive if=pflash,file=Build/OvmfIa32/DEBUG_GCC5/FV/OVMF_CODE.fd \
-drive if=pflash,file=Build/OvmfIa32/DEBUG_GCC5/FV/OVMF_VARS.fd


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108940): https://edk2.groups.io/g/devel/message/108940
Mute This Topic: https://groups.io/mt/99847923/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-