Xen ACPI tables support in OVMF #1

GavinGuan · 2012-07-05T09:23:46Z

Xen ACPI tables support in OVMF. Code files modified are :
AcpiPlatformDxe/AcpiPlatform.c
AcpiPlatformDxe/Xen.c

jljusten · 2012-08-28T06:27:52Z

This was committed in svn

I. There are at least three locations in OvmfPkg that manipulate the PMBA and related PIIX4 registers. 1. MiscInitialization() [OvmfPkg/PlatformPei/Platform.c] module type: PEIM -- Pre-EFI Initialization Module (a) currently sets the PMBA only: 00.01.3 / 0x40 bits [15:6] 2. AcpiTimerLibConstructor() [OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.c] module type: BASE -- probably callable anywhere after PEI (a) sets the PMBA if needed: 00.01.3 / 0x40 bits [15:6] (b) sets PCICMD/IOSE if needed: 00.01.3 / 0x04 bit 0 (c) sets PMREGMISC/PMIOSE: 00.01.3 / 0x80 bit 0 3. AcpiInitialization() [OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c] module type: DXE_DRIVER -- Driver eXecution Environment (a) sets SCI_EN, which depends on correct PMBA setting from earlier ( The relative order of #1 and #3 is dictated minimally by their module types. Said relative order can be verified with the boot log: 27 Loading PEIM at 0x00000822320 EntryPoint=0x00000822580 PlatformPei.efi 28 Platform PEIM Loaded 1259 PlatformBdsInit 1270 PlatformBdsPolicyBehavior Line 28 is printed by InitializePlatform() [OvmfPkg/PlatformPei/Platform.c] which is the entry point of that module. The other two lines are printed by the corresponding functions in "OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c". ) Currently #2 (AcpiTimerLibConstructor()) is called in a random spot (whenever it gets loaded from the firmware image) and masks the insufficient setup in #1. We shouldn't depend on that, PEI should finish with IO space being fully accessibe. In addition, PEI should program the same PMBA value as AcpiTimerLib. II. The PEI change notwithstanding, AcpiTimerLib should stay defensive and ensure proper PM configuration for itself (either by confirming or by doing). III. Considering a possible cleanup/unification of #2 and #3: timer functions relying on AcpiTimerLibConstructor(), - MicroSecondDelay() - NanoSecondDelay() - GetPerformanceCounter() - GetPerformanceCounterProperties() - GetTimeInNanoSecond() may be called before #3 is reached (in Boot Device Selection phase), so we should not move the initialization from #2 to #3. (Again, AcpiTimerLib should contain its own setup.) We should also not move #3 to an earlier phase -- SCI_EN is premature unless we're about to boot real soon ("enable generation of SCI upon assertion of PWRBTN_STS, LID_STS, THRM_STS, or GPI_STS bits"). Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13722 6f19259b-4bc3-4df7-8a09-765794883524

…tions in __aeabi_uread4 Change __aeabi_uread4 from: ldrb r2, [r0, #1] ldrb r1, [r0] (...) to: ldrb r1, [r0] ldrb r2, [r0, #1] (...) This change is a workaround to handle correctly __aeabi_uread4 on ARM Versatile Express RTSM. It should not have any major consequence on the other ARM platforms. git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@12481 6f19259b-4bc3-4df7-8a09-765794883524

I. There are at least three locations in OvmfPkg that manipulate the PMBA and related PIIX4 registers. 1. MiscInitialization() [OvmfPkg/PlatformPei/Platform.c] module type: PEIM -- Pre-EFI Initialization Module (a) currently sets the PMBA only: 00.01.3 / 0x40 bits [15:6] 2. AcpiTimerLibConstructor() [OvmfPkg/Library/AcpiTimerLib/AcpiTimerLib.c] module type: BASE -- probably callable anywhere after PEI (a) sets the PMBA if needed: 00.01.3 / 0x40 bits [15:6] (b) sets PCICMD/IOSE if needed: 00.01.3 / 0x04 bit 0 (c) sets PMREGMISC/PMIOSE: 00.01.3 / 0x80 bit 0 3. AcpiInitialization() [OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c] module type: DXE_DRIVER -- Driver eXecution Environment (a) sets SCI_EN, which depends on correct PMBA setting from earlier ( The relative order of #1 and tianocore#3 is dictated minimally by their module types. Said relative order can be verified with the boot log: 27 Loading PEIM at 0x00000822320 EntryPoint=0x00000822580 PlatformPei.efi 28 Platform PEIM Loaded 1259 PlatformBdsInit 1270 PlatformBdsPolicyBehavior Line 28 is printed by InitializePlatform() [OvmfPkg/PlatformPei/Platform.c] which is the entry point of that module. The other two lines are printed by the corresponding functions in "OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c". ) Currently tianocore#2 (AcpiTimerLibConstructor()) is called in a random spot (whenever it gets loaded from the firmware image) and masks the insufficient setup in #1. We shouldn't depend on that, PEI should finish with IO space being fully accessibe. In addition, PEI should program the same PMBA value as AcpiTimerLib. II. The PEI change notwithstanding, AcpiTimerLib should stay defensive and ensure proper PM configuration for itself (either by confirming or by doing). III. Considering a possible cleanup/unification of tianocore#2 and tianocore#3: timer functions relying on AcpiTimerLibConstructor(), - MicroSecondDelay() - NanoSecondDelay() - GetPerformanceCounter() - GetPerformanceCounterProperties() - GetTimeInNanoSecond() may be called before tianocore#3 is reached (in Boot Device Selection phase), so we should not move the initialization from tianocore#2 to tianocore#3. (Again, AcpiTimerLib should contain its own setup.) We should also not move tianocore#3 to an earlier phase -- SCI_EN is premature unless we're about to boot real soon ("enable generation of SCI upon assertion of PWRBTN_STS, LID_STS, THRM_STS, or GPI_STS bits"). Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com> git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13722 6f19259b-4bc3-4df7-8a09-765794883524

… issues came up related to vs2013 build and caused the build to fail. Vs2013 issue #1: warning message about uninitialized variables or pointers like this: s:\incbld\ia32\intelframeworkmodulepkg\bus\isa\isabusdxe\isabus.c(395) : warning C4701: potentially uninitialized local variable 'DevicePathData' used s:\incbld\ia32\intelframeworkmodulepkg\bus\isa\isabusdxe\isabus.c(395) : warning C4703: potentially uninitialized local pointer variable 'DevicePathData' used LINK : fatal error LNK1257: code generation failed The following online messages shows discussions related to this vs2013 issue and how Microsoft engineer responded. They suggest a work around by adding the initialization for the variables. https://connect.microsoft.com/VisualStudio/feedback/details/816730/bogus-warning-from-vs-2013 Vs2013 issue #2: C:\Program Files\Windows Kits\8.1\include\um\winnt.h(5105) : error C2220: warning treated as error - no 'object' file generated C:\Program Files\Windows Kits\8.1\include\um\winnt.h(5105) : warning C4005: 'InterlockedCompareExchange64' : macro redefinition This happened for Nt32Pkg. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Wang, Yu <yu.wang@intel.com> Reviewed-by: Gao, Liming <liming.gao@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15722 6f19259b-4bc3-4df7-8a09-765794883524

Example: Shell> echo "You are ^#1!" # Testing echo You are #1! Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chris Phillips <chrisp@hp.com> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@15871 6f19259b-4bc3-4df7-8a09-765794883524

XenStore is a key/value database, which is running on another virtual machine. It can be accessed through shared memory. This is a client implementation. Change in V3: - moving xs_wire.h from patch #1 to this patch - fix return value of XenStoreListDirectory - Use a timeout to print a debug message if the other side of the xenstore ring does not notify through the event channel. This is done with the new XenStoreWaitForEvent function. - Have XenStoreReadReply check status of XenStoreProcessMessage and return an error if needed. - Have XenStoreTalkv return the status of XenStoreReadReply. - Have a loop to check for the quiescent of the response ring in the XenStoreInitComms function. (with a timeout of 5 seconds) - use the recently introduced XenStore 'closing' feature. Change in V2: - Change comment style, from freebsd to ovmf - Fix type of EventChannel - Fix debug print, no more cast - Implement XenStoreDeinit. - Clean up comments - Fix few codding style issue - Add FAIL xenstore status value. Origin: FreeBSD 10.0 License: This patch adds several files under the MIT licence. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: Jordan Justen <jordan.l.justen@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16267 6f19259b-4bc3-4df7-8a09-765794883524

Some compilers requires an empty line at the end of the file. ARM compiler version 5 is one of these compilers: error #1-D: last line of file ends without a newline Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Olivier Martin <olivier.martin@arm.com> Reviewed-by: Feng Tian <feng.tian@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16918 6f19259b-4bc3-4df7-8a09-765794883524

Some compilers requires an empty line at the end of the file. ARM compiler version 5 is one of these compilers: error #1-D: last line of file ends without a newline (Sync patch r16918 from main trunk.) Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Olivier Martin <olivier.martin@arm.com> Reviewed-by: Feng Tian <feng.tian@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/branches/UDK2014.SP1@16996 6f19259b-4bc3-4df7-8a09-765794883524

PeiCore hang when loads a PEIM whose section alignment requirement is 0x40 but the actual base address is 0x20 aligned. The issue is caused by the following facts, in order: 1. GCC49 requires the section alignment of .data to be 0x40. So a new link script gcc4.9-ld-script was added for GCC49 to specify the 0x40 alignment. 2. GenFw tool was enhanced to sync ELF's section alignment to PE header. Before the enhancement, the section alignment of converted PE image always equals to 0x20. If only with #1 change, GCC49 build image won't hang in PeiCore because the converted PE image still claims 0x20 section alignment which is aligned to the align setting set in FDF file. But later with #2 change, the converted PE image starts to claims 0x40 section alignment, while build tool still puts the PEIM in 0x20 aligned address, resulting the PeCoffLoaderLoadImage() reports IMAGE_ERROR_INVALID_SECTION_ALIGNMENT error. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Reviewed-by: Liming Gao <liming.gao@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@17902 6f19259b-4bc3-4df7-8a09-765794883524

There is a hidden bug for below code: (1 << BaseAddressAlignment) & *BlockEntrySize From disassembly code, we can see the literal number 1 will be treated as INT32 by compiler by default, and we'll get 0xFFFFFFFF80000000 when BaseAddressAlignment is equal to 31. So we will always get 31 when alignment is larger than 31. if ((1 << BaseAddressAlignment) & *BlockEntrySize) { 5224: f9404be0 ldr x0, [sp,#144] 5228: 2a0003e1 mov w1, w0 522c: 52800020 mov w0, #0x1 // #1 5230: 1ac12000 lsl w0, w0, w1 5234: 93407c01 sxtw x1, w0 The bug can be replayed on QEMU AARCH64; by adding some debug print, we can see lots of level 1 tables created (for block of 1GB) even when the region is large enough to use 512GB block size. Use LowBitSet64() in BaseLib instead to fix the bug. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Heyi Guo <heyi.guo@linaro.org> Cc: Leif Lindholm <leif.lindholm@linaro.org> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18423 6f19259b-4bc3-4df7-8a09-765794883524

IpSb->Reconfig should not be set to TRUE to focal the reconfiguration during the policy changes from Static to DHCP. It's redundancy because the default router table and default addresses have been freed ahead ( Detailed see Ip4Config2OnPolicyChanged() function). Otherwise, the potential failure will appear if UseDefaultAddress configured. Reproduce steps see below: #1. Set policy to DHCP. #2. If DHCP process is not complete yet, then run one APP to invoke UDP4 Configure with "UseDefaultAddress = TRUE" (loop to call UDP4 Configure until Ip4Mode.IsConfigured changes to TRUE). #3. Even DHCP succeed but Ip4Mode.IsConfigured flag never set to TRUE Concrete analysis is as follows: In #1, the policy will be set to DHCP, and then Ip4Config2OnPolicyChanged() will be called. In this function, if "IpSb->Reconfig" flag is set to TRUE, the original "IpSb->DefaultInterface" will be abandoned/freed once the DHCP process finished. In #2, UDP4 Configure with "UseDefaultAddress = TRUE" is called, that means the default interface (IpSb->DefaultInterface) will be selected as current instance's interface. In #3, when DHCP process finished, the original DefaultInterface will be abandoned/freed because "IpSb->Reconfig" flag is true. Meanwhile, one new interface is assigned to "IpSb->DefaultInterface". This new interface is different to the original one assigned to the UDP4 Configured instance. So, even DHCP process succeed, the up caller will never have the chance to get it's truly status. Cc: Cohen Eugene <eugene@hp.com> Cc: Ye Ting <ting.ye@intel.com> Cc: Fu Siyuan <siyuan.fu@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com> Tested-by: Eugene Cohen <eugene@hp.com> Reviewed-by: Ye Ting <ting.ye@intel.com>

*v2: update the commit log and refine the code comments. There are three kinds of IKE Exchange process: #1. Initial Exchange #2. CREATE_CHILD_SA_Exchange #3. Information Exchange The IKE header "FLAG" update is incorrect in #2 and #3 exchange, which may cause the continue session failure. This patch is used to correct the updates of IKE header "FLAG" according the RFC4306 section 3.1. Cc: Ye Ting <ting.ye@intel.com> Cc: Fu Siyuan <siyuan.fu@intel.com> Cc: Zhang Lubo <lubo.zhang@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com> Reviewed-by: Ye Ting <ting.ye@intel.com>

This reverts commit 95fc5a8. The above commit causes several regression of "echo" command: 1. Double quotes are not being stripped from the final text. UEFI Shell 2.2 section 3.4.5 chops out the quotes. 2. Output redirection is not working as expected. Text is being redirected, but the ‘> …’ text should not be. 3. Inconsistent special character handling. For example, comments with # seem to be parsed out correctly, but handing of ^ is incorrect. In summary, ‘echo “You are ^#1” > t.txt’ results in the below content in t.txt: “You are ^#1” > t.txt Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jaben Carsey <jaben.carsey@intel.com> Reviewed-by: Tapan Shah <tapandshah@hpe.com>

This reverts commit 0fcf8d4. The above commit causes several regression of "echo" command: 1. Double quotes are not being stripped from the final text. UEFI Shell 2.2 section 3.4.5 chops out the quotes. 2. Output redirection is not working as expected. Text is being redirected, but the ‘> …’ text should not be. 3. Inconsistent special character handling. For example, comments with # seem to be parsed out correctly, but handing of ^ is incorrect. In summary, ‘echo “You are ^#1” > t.txt’ results in the below content in t.txt: “You are ^#1” > t.txt Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jaben Carsey <jaben.carsey@intel.com> Reviewed-by: Tapan Shah <tapandshah@hpe.com>

…mmandLib" This reverts commit c0bcd34. The above commit causes several regression of "echo" command: 1. Double quotes are not being stripped from the final text. UEFI Shell 2.2 section 3.4.5 chops out the quotes. 2. Output redirection is not working as expected. Text is being redirected, but the ‘> …’ text should not be. 3. Inconsistent special character handling. For example, comments with # seem to be parsed out correctly, but handing of ^ is incorrect. In summary, ‘echo “You are ^#1” > t.txt’ results in the below content in t.txt: “You are ^#1” > t.txt Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jaben Carsey <jaben.carsey@intel.com> Reviewed-by: Tapan Shah <tapandshah@hpe.com>

This reverts commit 95fc5a8. The above commit causes several regression of "echo" command: 1. Double quotes are not being stripped from the final text. UEFI Shell 2.2 section 3.4.5 chops out the quotes. 2. Output redirection is not working as expected. Text is being redirected, but the ‘> …’ text should not be. 3. Inconsistent special character handling. For example, comments with # seem to be parsed out correctly, but handing of ^ is incorrect. In summary, ‘echo “You are ^#1” > t.txt’ results in the below content in t.txt: “You are ^#1” > t.txt Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jaben Carsey <jaben.carsey@intel.com> Reviewed-by: Tapan Shah <tapandshah@hpe.com>

This reverts commit 0fcf8d4. The above commit causes several regression of "echo" command: 1. Double quotes are not being stripped from the final text. UEFI Shell 2.2 section 3.4.5 chops out the quotes. 2. Output redirection is not working as expected. Text is being redirected, but the ‘> …’ text should not be. 3. Inconsistent special character handling. For example, comments with # seem to be parsed out correctly, but handing of ^ is incorrect. In summary, ‘echo “You are ^#1” > t.txt’ results in the below content in t.txt: “You are ^#1” > t.txt Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jaben Carsey <jaben.carsey@intel.com> Reviewed-by: Tapan Shah <tapandshah@hpe.com>

…mmandLib" This reverts commit c0bcd34. The above commit causes several regression of "echo" command: 1. Double quotes are not being stripped from the final text. UEFI Shell 2.2 section 3.4.5 chops out the quotes. 2. Output redirection is not working as expected. Text is being redirected, but the ‘> …’ text should not be. 3. Inconsistent special character handling. For example, comments with # seem to be parsed out correctly, but handing of ^ is incorrect. In summary, ‘echo “You are ^#1” > t.txt’ results in the below content in t.txt: “You are ^#1” > t.txt Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com> Cc: Jaben Carsey <jaben.carsey@intel.com> Reviewed-by: Tapan Shah <tapandshah@hpe.com>

Commit bd3fc81 ("ShellPkg/App: Fix memory leak and save resources.", 2016-05-20) added a FreePool() call for Split->SplitStdIn, near end of the RunSplitCommand(), right after the same shell file was closed with CloseFile(). The argument was: > 1) RunSplitCommand() allocates the initial SplitStdOut via > CreateFileInterfaceMem(). Free SplitStdIn after the swap to fix > the memory leak. There is no memory leak actually, and the FreePool() call in question constitutes a double-free: (a) This is how the handle is established: ConvertEfiFileProtocolToShellHandle ( CreateFileInterfaceMem (Unicode), NULL ); CreateFileInterfaceMem() allocates an EFI_FILE_PROTOCOL_MEM object and populates it fully. ConvertEfiFileProtocolToShellHandle() allocates some administrative structures and links the EFI_FILE_PROTOCOL_MEM object into "mFileHandleList". (b) EFI_SHELL_PROTOCOL.CloseFile() is required to close the SHELL_FILE_HANDLE and to release all associated data. Accordingly, near the end of RunSplitCommand(), we have: EfiShellClose() ShellFileHandleRemove() // // undoes the effects of ConvertEfiFileProtocolToShellHandle() // ConvertShellHandleToEfiFileProtocol() // // note that this does not adjust the pointer value; it's a pure // type cast // FileHandleClose() FileInterfaceMemClose() // // tears down EFI_FILE_PROTOCOL_MEM completely, undoing the // effects of CreateFileInterfaceMem () // The FreePool() call added by bd3fc81 conflicts with SHELL_FREE_NON_NULL(This); in FileInterfaceMemClose(), so remove it. This error can be reproduced for example with: > Shell> map | more > 'more' is not recognized as an internal or external command, operable > program, or script file. which triggers: > ASSERT MdeModulePkg/Core/Dxe/Mem/Pool.c(624): CR has Bad Signature with the following stack dump: > #0 0x000000007f6dc094 in CpuDeadLoop () at > MdePkg/Library/BaseLib/CpuDeadLoop.c:37 > #1 0x000000007f6dd1b4 in DebugAssert (FileName=0x7f6ed9f0 > "MdeModulePkg/Core/Dxe/Mem/Pool.c", LineNumber=624, > Description=0x7f6ed9d8 "CR has Bad Signature") at > OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c:153 > #2 0x000000007f6d075d in CoreFreePoolI (Buffer=0x7e232c98, > PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:624 > tianocore#3 0x000000007f6d060e in CoreInternalFreePool (Buffer=0x7e232c98, > PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:529 > tianocore#4 0x000000007f6d0648 in CoreFreePool (Buffer=0x7e232c98) at > MdeModulePkg/Core/Dxe/Mem/Pool.c:552 > tianocore#5 0x000000007d49fbf8 in FreePool (Buffer=0x7e232c98) at > MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c:818 > tianocore#6 0x000000007d4875c3 in RunSplitCommand (CmdLine=0x7d898398, > StdIn=0x0, StdOut=0x0) at ShellPkg/Application/Shell/Shell.c:1813 > tianocore#7 0x000000007d487d59 in ProcessNewSplitCommandLine > (CmdLine=0x7d898398) at ShellPkg/Application/Shell/Shell.c:2121 > tianocore#8 0x000000007d488937 in RunShellCommand (CmdLine=0x7e233018, > CommandStatus=0x0) at ShellPkg/Application/Shell/Shell.c:2670 > tianocore#9 0x000000007d488b0b in RunCommand (CmdLine=0x7e233018) at > ShellPkg/Application/Shell/Shell.c:2732 > tianocore#10 0x000000007d4867c8 in DoShellPrompt () at > ShellPkg/Application/Shell/Shell.c:1349 > tianocore#11 0x000000007d48524d in UefiMain (ImageHandle=0x7e24c898, > SystemTable=0x7f5b6018) at ShellPkg/Application/Shell/Shell.c:631 Cc: Jaben Carsey <jaben.carsey@intel.com> Cc: Marvin Häuser <Marvin.Haeuser@outlook.com> Cc: Qiu Shumin <shumin.qiu@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Fixes: bd3fc81 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Commit bd3fc81 ("ShellPkg/App: Fix memory leak and save resources.", 2016-05-20) added a FreePool() call for Split->SplitStdIn, near end of the RunSplitCommand(), right after the same shell file was closed with CloseFile(). The argument was: > 1) RunSplitCommand() allocates the initial SplitStdOut via > CreateFileInterfaceMem(). Free SplitStdIn after the swap to fix > the memory leak. There is no memory leak actually, and the FreePool() call in question constitutes a double-free: (a) This is how the handle is established: ConvertEfiFileProtocolToShellHandle ( CreateFileInterfaceMem (Unicode), NULL ); CreateFileInterfaceMem() allocates an EFI_FILE_PROTOCOL_MEM object and populates it fully. ConvertEfiFileProtocolToShellHandle() allocates some administrative structures and links the EFI_FILE_PROTOCOL_MEM object into "mFileHandleList". (b) EFI_SHELL_PROTOCOL.CloseFile() is required to close the SHELL_FILE_HANDLE and to release all associated data. Accordingly, near the end of RunSplitCommand(), we have: EfiShellClose() ShellFileHandleRemove() // // undoes the effects of ConvertEfiFileProtocolToShellHandle() // ConvertShellHandleToEfiFileProtocol() // // note that this does not adjust the pointer value; it's a pure // type cast // FileHandleClose() FileInterfaceMemClose() // // tears down EFI_FILE_PROTOCOL_MEM completely, undoing the // effects of CreateFileInterfaceMem () // The FreePool() call added by bd3fc81 conflicts with SHELL_FREE_NON_NULL(This); in FileInterfaceMemClose(), so remove it. This error can be reproduced for example with: > Shell> map | more > 'more' is not recognized as an internal or external command, operable > program, or script file. which triggers: > ASSERT MdeModulePkg/Core/Dxe/Mem/Pool.c(624): CR has Bad Signature with the following stack dump: > #0 0x000000007f6dc094 in CpuDeadLoop () at > MdePkg/Library/BaseLib/CpuDeadLoop.c:37 > #1 0x000000007f6dd1b4 in DebugAssert (FileName=0x7f6ed9f0 > "MdeModulePkg/Core/Dxe/Mem/Pool.c", LineNumber=624, > Description=0x7f6ed9d8 "CR has Bad Signature") at > OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c:153 > #2 0x000000007f6d075d in CoreFreePoolI (Buffer=0x7e232c98, > PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:624 > #3 0x000000007f6d060e in CoreInternalFreePool (Buffer=0x7e232c98, > PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:529 > #4 0x000000007f6d0648 in CoreFreePool (Buffer=0x7e232c98) at > MdeModulePkg/Core/Dxe/Mem/Pool.c:552 > #5 0x000000007d49fbf8 in FreePool (Buffer=0x7e232c98) at > MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c:818 > #6 0x000000007d4875c3 in RunSplitCommand (CmdLine=0x7d898398, > StdIn=0x0, StdOut=0x0) at ShellPkg/Application/Shell/Shell.c:1813 > #7 0x000000007d487d59 in ProcessNewSplitCommandLine > (CmdLine=0x7d898398) at ShellPkg/Application/Shell/Shell.c:2121 > #8 0x000000007d488937 in RunShellCommand (CmdLine=0x7e233018, > CommandStatus=0x0) at ShellPkg/Application/Shell/Shell.c:2670 > #9 0x000000007d488b0b in RunCommand (CmdLine=0x7e233018) at > ShellPkg/Application/Shell/Shell.c:2732 > #10 0x000000007d4867c8 in DoShellPrompt () at > ShellPkg/Application/Shell/Shell.c:1349 > #11 0x000000007d48524d in UefiMain (ImageHandle=0x7e24c898, > SystemTable=0x7f5b6018) at ShellPkg/Application/Shell/Shell.c:631 Cc: Jaben Carsey <jaben.carsey@intel.com> Cc: Marvin Häuser <Marvin.Haeuser@outlook.com> Cc: Qiu Shumin <shumin.qiu@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Fixes: bd3fc81 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> Reviewed-by: Marvin Häuser <Marvin.Haeuser@outlook.com>

Commit bd3fc81 ("ShellPkg/App: Fix memory leak and save resources.", 2016-05-20) added a FreePool() call for Split->SplitStdIn, near end of the RunSplitCommand(), right after the same shell file was closed with CloseFile(). The argument was: > 1) RunSplitCommand() allocates the initial SplitStdOut via > CreateFileInterfaceMem(). Free SplitStdIn after the swap to fix > the memory leak. There is no memory leak actually, and the FreePool() call in question constitutes a double-free: (a) This is how the handle is established: ConvertEfiFileProtocolToShellHandle ( CreateFileInterfaceMem (Unicode), NULL ); CreateFileInterfaceMem() allocates an EFI_FILE_PROTOCOL_MEM object and populates it fully. ConvertEfiFileProtocolToShellHandle() allocates some administrative structures and links the EFI_FILE_PROTOCOL_MEM object into "mFileHandleList". (b) EFI_SHELL_PROTOCOL.CloseFile() is required to close the SHELL_FILE_HANDLE and to release all associated data. Accordingly, near the end of RunSplitCommand(), we have: EfiShellClose() ShellFileHandleRemove() // // undoes the effects of ConvertEfiFileProtocolToShellHandle() // ConvertShellHandleToEfiFileProtocol() // // note that this does not adjust the pointer value; it's a pure // type cast // FileHandleClose() FileInterfaceMemClose() // // tears down EFI_FILE_PROTOCOL_MEM completely, undoing the // effects of CreateFileInterfaceMem () // The FreePool() call added by bd3fc81 conflicts with SHELL_FREE_NON_NULL(This); in FileInterfaceMemClose(), so remove it. This error can be reproduced for example with: > Shell> map | more > 'more' is not recognized as an internal or external command, operable > program, or script file. which triggers: > ASSERT MdeModulePkg/Core/Dxe/Mem/Pool.c(624): CR has Bad Signature with the following stack dump: > #0 0x000000007f6dc094 in CpuDeadLoop () at > MdePkg/Library/BaseLib/CpuDeadLoop.c:37 > #1 0x000000007f6dd1b4 in DebugAssert (FileName=0x7f6ed9f0 > "MdeModulePkg/Core/Dxe/Mem/Pool.c", LineNumber=624, > Description=0x7f6ed9d8 "CR has Bad Signature") at > OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c:153 > #2 0x000000007f6d075d in CoreFreePoolI (Buffer=0x7e232c98, > PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:624 > #3 0x000000007f6d060e in CoreInternalFreePool (Buffer=0x7e232c98, > PoolType=0x7f6bc1c4) at MdeModulePkg/Core/Dxe/Mem/Pool.c:529 > #4 0x000000007f6d0648 in CoreFreePool (Buffer=0x7e232c98) at > MdeModulePkg/Core/Dxe/Mem/Pool.c:552 > #5 0x000000007d49fbf8 in FreePool (Buffer=0x7e232c98) at > MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c:818 > #6 0x000000007d4875c3 in RunSplitCommand (CmdLine=0x7d898398, > StdIn=0x0, StdOut=0x0) at ShellPkg/Application/Shell/Shell.c:1813 > #7 0x000000007d487d59 in ProcessNewSplitCommandLine > (CmdLine=0x7d898398) at ShellPkg/Application/Shell/Shell.c:2121 > #8 0x000000007d488937 in RunShellCommand (CmdLine=0x7e233018, > CommandStatus=0x0) at ShellPkg/Application/Shell/Shell.c:2670 > #9 0x000000007d488b0b in RunCommand (CmdLine=0x7e233018) at > ShellPkg/Application/Shell/Shell.c:2732 > #10 0x000000007d4867c8 in DoShellPrompt () at > ShellPkg/Application/Shell/Shell.c:1349 > #11 0x000000007d48524d in UefiMain (ImageHandle=0x7e24c898, > SystemTable=0x7f5b6018) at ShellPkg/Application/Shell/Shell.c:631 Cc: Jaben Carsey <jaben.carsey@intel.com> Cc: Marvin Häuser <Marvin.Haeuser@outlook.com> Cc: Qiu Shumin <shumin.qiu@intel.com> Cc: Ruiyu Ni <ruiyu.ni@intel.com> Fixes: bd3fc81 Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com> Reviewed-by: Marvin Häuser <Marvin.Haeuser@outlook.com> (cherry picked from commit 227fe49)

Root cause: 1. Before DisableReadonlyPageWriteProtect() is called, the return address (tianocore#1) is pushed in shadow stack. 2. CET is disabled. 3. DisableReadonlyPageWriteProtect() returns to tianocore#1. 4. Page table is modified. 5. EnableReadonlyPageWriteProtect() is called, but the return address (tianocore#2) is not pushed in shadow stack. 6. CET is enabled. 7. EnableReadonlyPageWriteProtect() returns to tianocore#2. #CP exception happens because the actual return address (tianocore#2) doesn't match the return address stored in shadow stack (tianocore#1). Analysis: Shadow stack will stop update after CET disable (DisableCet() in DisableReadOnlyPageWriteProtect), but normal smi stack will be continue updated with the function called and return (DisableReadOnlyPageWriteProtect & EnableReadOnlyPageWriteProtect), thus leading stack mismatch after CET re-enabled (EnableCet() in EnableReadOnlyPageWriteProtect). According SDM Vol 3, 6.15-Control Protection Exception: Normal smi stack and shadow stack must be matched when CET enable, otherwise CP Exception will happen, which is caused by a near RET instruction. CET is disabled in DisableCet(), while can be enabled in EnableCet(). This way won't cause the problem because they are implemented in a way that return address of DisableCet() is poped out from shadow stack (Incsspq performs a pop to increases the shadow stack) and EnableCet() doesn't use "RET" but "JMP" to return to caller. So calling EnableCet() and DisableCet() doesn't have the same issue as calling DisableReadonlyPageWriteProtect() and EnableReadonlyPageWriteProtect(). With above root cause & analysis, define below 2 macros instead of functions for WP & CET operation: WRITE_UNPROTECT_RO_PAGES (Wp, Cet) WRITE_PROTECT_RO_PAGES (Wp, Cet) Because DisableCet() & EnableCet() must be in the same function to avoid shadow stack and normal SMI stack mismatch. Note: WRITE_UNPROTECT_RO_PAGES () must be called pair with WRITE_PROTECT_RO_PAGES () in same function. Change-Id: I4e126697efcd8dbfb4887da034d8691bfca969e3 Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Zeng Star <star.zeng@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com>

Root cause: 1. Before DisableReadonlyPageWriteProtect() is called, the return address (#1) is pushed in shadow stack. 2. CET is disabled. 3. DisableReadonlyPageWriteProtect() returns to #1. 4. Page table is modified. 5. EnableReadonlyPageWriteProtect() is called, but the return address (#2) is not pushed in shadow stack. 6. CET is enabled. 7. EnableReadonlyPageWriteProtect() returns to #2. #CP exception happens because the actual return address (#2) doesn't match the return address stored in shadow stack (#1). Analysis: Shadow stack will stop update after CET disable (DisableCet() in DisableReadOnlyPageWriteProtect), but normal smi stack will be continue updated with the function called and return (DisableReadOnlyPageWriteProtect & EnableReadOnlyPageWriteProtect), thus leading stack mismatch after CET re-enabled (EnableCet() in EnableReadOnlyPageWriteProtect). According SDM Vol 3, 6.15-Control Protection Exception: Normal smi stack and shadow stack must be matched when CET enable, otherwise CP Exception will happen, which is caused by a near RET instruction. CET is disabled in DisableCet(), while can be enabled in EnableCet(). This way won't cause the problem because they are implemented in a way that return address of DisableCet() is poped out from shadow stack (Incsspq performs a pop to increases the shadow stack) and EnableCet() doesn't use "RET" but "JMP" to return to caller. So calling EnableCet() and DisableCet() doesn't have the same issue as calling DisableReadonlyPageWriteProtect() and EnableReadonlyPageWriteProtect(). With above root cause & analysis, define below 2 macros instead of functions for WP & CET operation: WRITE_UNPROTECT_RO_PAGES (Wp, Cet) WRITE_PROTECT_RO_PAGES (Wp, Cet) Because DisableCet() & EnableCet() must be in the same function to avoid shadow stack and normal SMI stack mismatch. Note: WRITE_UNPROTECT_RO_PAGES () must be called pair with WRITE_PROTECT_RO_PAGES () in same function. Change-Id: I4e126697efcd8dbfb4887da034d8691bfca969e3 Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Zeng Star <star.zeng@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4534 Bug Details: PixieFail Bug tianocore#1 CVE-2023-45229 CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N CWE-125 Out-of-bounds Read Change Overview: Introduce Dhcp6SeekInnerOptionSafe which performs checks before seeking the Inner Option from a DHCP6 Option. > > EFI_STATUS > Dhcp6SeekInnerOptionSafe ( > IN UINT16 IaType, > IN UINT8 *Option, > IN UINT32 OptionLen, > OUT UINT8 **IaInnerOpt, > OUT UINT16 *IaInnerLen > ); > Lots of code cleanup to improve code readability. Cc: Saloni Kasbekar <saloni.kasbekar@intel.com> Cc: Zachary Clark-williams <zachary.clark-williams@intel.com> Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>

@font-face

--_000_MN6PR11MB82443B73244CEF84481629F08C7A2MN6PR11MB8244namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable All, This mail is to bring another approach to solve the stack-overflow due to n= ested interrupts. Michael solved this problem in OVMF through NestedInterru= ptTplLib. I made a draft patch as attached "DxeCore.diff". The patch simply to avoid = the interrupt in enable state when TPL is dropped to the interrupted TPL. T= he interrupt will be enabled later through "IRET". So, a timer driver has two ways to implement its timer interrupt handler: 1. Do raise/restore TPL in the TimerInterruptHandler(). But call the A= PIs in NestedInterruptTplLib. 2. Do not raise/restore TPL in the TimerInterruptHandler(). So that on= ly DxeCore restores the TPL. And when DxeCore restores the TPL, the interru= pt is not enabled when TPL is dropped to the interrupted TPL (as it will be= enabled later by "IRET"). Implementing the logic in DxeCore does not prevent the TimerInterruptHandle= r() from implementing the way tianocore#1. Agree on the draft patch? My 2nd question is can we set a rule that TimerInterruptHandler() should NO= T restore TPL so that way tianocore#2 (changing DxeCore) is enough to solve the stac= k overflow issue due to nested interrupts. I was aware of the discussion between Laszlo and Michael in end of 2022 but= never dig deeply as today into this problem. I really appreciate the long discussion in the bugzilla (https://bugzilla.t= ianocore.org/show_bug.cgi?id=3D4162) and comments in NestedInterruptTplLib.= I learned a lot from them and they are quite interesting! Thanks, Ray -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#114369): https://edk2.groups.io/g/devel/message/114369 Mute This Topic: https://groups.io/mt/103950154/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076= /xyzzy [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- <html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="ZH-CN" link="#0563C1" vlink="#954F72" style="word-wrap:break-word;text-justify-trim:punctuation"> <div class="WordSection1"> All,<o:p></o:p> This mail is to bring another approach to solve the stack-overflow due to nested interrupts. Michael solved this problem in OVMF through NestedInterruptTplLib.<o:p></o:p> <o:p> </o:p> I made a draft patch as attached “DxeCore.diff”. The patch simply to avoid the interrupt in enable state when TPL is dropped to the interrupted TPL. The interrupt will be enabled later through “IRET”.<o:p></o:p> <o:p> </o:p> So, a timer driver has two ways to implement its timer interrupt handler:<o:p></o:p> <![if !supportLists]>1.      <![endif]>Do raise/restore TPL in the TimerInterruptHandler(). But call the APIs in NestedInterruptTplLib.<o:p></o:p> <![if !supportLists]>2.      <![endif]>Do not raise/restore TPL in the TimerInterruptHandler(). So that only DxeCore restores the TPL. And when DxeCore restores the TPL, the interrupt is not enabled when TPL is dropped to the interrupted TPL (as it will be enabled later by “IRET”).<o:p></o:p> <o:p> </o:p> <o:p> </o:p> Implementing the logic in DxeCore does not prevent the TimerInterruptHandler() from implementing the way tianocore#1.<o:p></o:p> Agree on the draft patch?<o:p></o:p> <o:p> </o:p> My 2nd question is can we set a rule that TimerInterruptHandler() should NOT restore TPL so that way tianocore#2 (changing DxeCore) is enough to solve the stack overflow issue due to nested interrupts.<o:p></o:p> <o:p> </o:p> I was aware of the discussion between Laszlo and Michael in end of 2022 but never dig deeply as today into this problem.<o:p></o:p> I really appreciate the long discussion in the bugzilla (<a href="https://bugzilla.tianocore.org/show_bug.cgi?id=4162">https://bugzilla.tianocore.org/show_bug.cgi?id=4162</a>) and comments in NestedInterruptTplLib. I learned a lot from them and they are quite interesting!<o:p></o:p> <div> <o:p> </o:p> Thanks,<o:p></o:p> </div> Ray<o:p></o:p> </div> </body> </html> <div width="1" style="color:white;clear:both">_._,_._,_</div> <hr> Groups.io Links: You receive all messages sent to this group. <a target="_blank" href="https://edk2.groups.io/g/devel/message/114369">View/Reply Online (#114369)</a> | | <a target="_blank" href="https://groups.io/mt/103950154/1787277">Mute This Topic</a> | <a href="https://edk2.groups.io/g/devel/post">New Topic</a> <a href="https://edk2.groups.io/g/devel/editsub/1787277">Your Subscription</a> | <a href="mailto:devel+owner@edk2.groups.io">Contact Group Owner</a> | <a href="https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy">Unsubscribe</a> [importer@patchew.org] <div width="1" style="color:white;clear:both">_._,_._,_</div> Message-Id: <MN6PR11MB82443B73244CEF84481629F08C7A2@MN6PR11MB8244.namprd11.prod.outlook.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4534 Bug Details: PixieFail Bug tianocore#1 CVE-2023-45229 CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N CWE-125 Out-of-bounds Read Change Overview: Introduce Dhcp6SeekInnerOptionSafe which performs checks before seeking the Inner Option from a DHCP6 Option. > > EFI_STATUS > Dhcp6SeekInnerOptionSafe ( > IN UINT16 IaType, > IN UINT8 *Option, > IN UINT32 OptionLen, > OUT UINT8 **IaInnerOpt, > OUT UINT16 *IaInnerLen > ); > Lots of code cleanup to improve code readability. Cc: Saloni Kasbekar <saloni.kasbekar@intel.com> Cc: Zachary Clark-williams <zachary.clark-williams@intel.com> Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4534 Bug Details: PixieFail Bug #1 CVE-2023-45229 CVSS 6.5 : CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N CWE-125 Out-of-bounds Read Change Overview: Introduce Dhcp6SeekInnerOptionSafe which performs checks before seeking the Inner Option from a DHCP6 Option. > > EFI_STATUS > Dhcp6SeekInnerOptionSafe ( > IN UINT16 IaType, > IN UINT8 *Option, > IN UINT32 OptionLen, > OUT UINT8 **IaInnerOpt, > OUT UINT16 *IaInnerLen > ); > Lots of code cleanup to improve code readability. Cc: Saloni Kasbekar <saloni.kasbekar@intel.com> Cc: Zachary Clark-williams <zachary.clark-williams@intel.com> Signed-off-by: Doug Flick [MSFT] <doug.edk2@gmail.com> Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>

This could be a lot better, but it's good enough for writing a timer arch protocol that will use the HDEC. Fixes Issue tianocore#1. Introduces Issue tianocore#11 Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com>

Root cause: 1. Before DisableReadonlyPageWriteProtect() is called, the return address (tianocore#1) is pushed in shadow stack. 2. CET is disabled. 3. DisableReadonlyPageWriteProtect() returns to tianocore#1. 4. Page table is modified. 5. EnableReadonlyPageWriteProtect() is called, but the return address (tianocore#2) is not pushed in shadow stack. 6. CET is enabled. 7. EnableReadonlyPageWriteProtect() returns to tianocore#2. #CP exception happens because the actual return address (tianocore#2) doesn't match the return address stored in shadow stack (tianocore#1). Analysis: Shadow stack will stop update after CET disable (DisableCet() in DisableReadOnlyPageWriteProtect), but normal smi stack will be continue updated with the function called and return (DisableReadOnlyPageWriteProtect & EnableReadOnlyPageWriteProtect), thus leading stack mismatch after CET re-enabled (EnableCet() in EnableReadOnlyPageWriteProtect). According SDM Vol 3, 6.15-Control Protection Exception: Normal smi stack and shadow stack must be matched when CET enable, otherwise CP Exception will happen, which is caused by a near RET instruction. CET is disabled in DisableCet(), while can be enabled in EnableCet(). This way won't cause the problem because they are implemented in a way that return address of DisableCet() is poped out from shadow stack (Incsspq performs a pop to increases the shadow stack) and EnableCet() doesn't use "RET" but "JMP" to return to caller. So calling EnableCet() and DisableCet() doesn't have the same issue as calling DisableReadonlyPageWriteProtect() and EnableReadonlyPageWriteProtect(). With above root cause & analysis, define below 2 macros instead of functions for WP & CET operation: WRITE_UNPROTECT_RO_PAGES (Wp, Cet) WRITE_PROTECT_RO_PAGES (Wp, Cet) Because DisableCet() & EnableCet() must be in the same function to avoid shadow stack and normal SMI stack mismatch. Note: WRITE_UNPROTECT_RO_PAGES () must be called pair with WRITE_PROTECT_RO_PAGES () in same function. Change-Id: I4e126697efcd8dbfb4887da034d8691bfca969e3 Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Zeng Star <star.zeng@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Ray Ni <ray.ni@intel.com> Reviewed-by: Eric Dong <eric.dong@intel.com>

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

This patch does not impact functionality. It aims to clarify the synchronization flow between the BSP and APs to enhance code readability and understanding: Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all cases. Steps tianocore#1 and tianocore#2 are additional requirements if the MmCpuSyncModeTradition mode is selected. Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional requirements if the system needs to configure the MTRR. Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to support the mSmmDebugAgentSupport. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

This patch does not impact functionality. It aims to clarify the synchronization flow between the BSP and APs to enhance code readability and understanding: Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all cases. Steps tianocore#1 is additional requirements if the MmCpuSyncModeTradition mode is selected. Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional requirements if the system needs to configure the MTRR. Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to support the mSmmDebugAgentSupport. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

… func This patch is for PiSmmCpuDxeSmm driver to add one round wait/release sync for BSP and AP to perform the SMM CPU Platform Hook before executing MMI Handler: SmmCpuPlatformHookBeforeMmiHandler (). With the function, SMM CPU driver can perform the platform specific items after one round BSP and AP sync (to make sure all APs in SMI) and before the MMI handlers. After the change, steps tianocore#1 and tianocore#2 are additional requirements if the MmCpuSyncModeTradition mode is selected. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

This patch does not impact functionality. It aims to clarify the synchronization flow between the BSP and APs to enhance code readability and understanding: Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all cases. Steps tianocore#1 is additional requirements if the MmCpuSyncModeTradition mode is selected. Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional requirements if the system needs to configure the MTRR. Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to support the mSmmDebugAgentSupport. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

… func This patch is for PiSmmCpuDxeSmm driver to add one round wait/release sync for BSP and AP to perform the SMM CPU Platform Hook before executing MMI Handler: SmmCpuPlatformHookBeforeMmiHandler (). With the function, SMM CPU driver can perform the platform specific items after one round BSP and AP sync (to make sure all APs in SMI) and before the MMI handlers. After the change, steps tianocore#1 and tianocore#2 are additional requirements if the MmCpuSyncModeTradition mode is selected. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

This patch does not impact functionality. It aims to clarify the synchronization flow between the BSP and APs to enhance code readability and understanding: Steps #6 and #11 are the basic synchronization requirements for all cases. Steps #1 is additional requirements if the MmCpuSyncModeTradition mode is selected. Steps #1, #2, #3, #4, #5, #7, #8, #9, and #10 are additional requirements if the system needs to configure the MTRR. Steps #9 and #10 are additional requirements if the system needs to support the mSmmDebugAgentSupport. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

… func This patch is for PiSmmCpuDxeSmm driver to add one round wait/release sync for BSP and AP to perform the SMM CPU Platform Hook before executing MMI Handler: SmmCpuPlatformHookBeforeMmiHandler (). With the function, SMM CPU driver can perform the platform specific items after one round BSP and AP sync (to make sure all APs in SMI) and before the MMI handlers. After the change, steps #1 and #2 are additional requirements if the MmCpuSyncModeTradition mode is selected. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

This patch does not impact functionality. It aims to clarify the synchronization flow between the BSP and APs to enhance code readability and understanding: Steps tianocore#6 and tianocore#11 are the basic synchronization requirements for all cases. Steps tianocore#1 is additional requirements if the MmCpuSyncModeTradition mode is selected. Steps tianocore#1, tianocore#2, tianocore#3, tianocore#4, tianocore#5, tianocore#7, tianocore#8, tianocore#9, and tianocore#10 are additional requirements if the system needs to configure the MTRR. Steps tianocore#9 and tianocore#10 are additional requirements if the system needs to support the mSmmDebugAgentSupport. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

… func This patch is for PiSmmCpuDxeSmm driver to add one round wait/release sync for BSP and AP to perform the SMM CPU Platform Hook before executing MMI Handler: SmmCpuPlatformHookBeforeMmiHandler (). With the function, SMM CPU driver can perform the platform specific items after one round BSP and AP sync (to make sure all APs in SMI) and before the MMI handlers. After the change, steps tianocore#1 and tianocore#2 are additional requirements if the MmCpuSyncModeTradition mode is selected. Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>

GavinGuan added 7 commits July 5, 2012 16:46

support Xen acpi tables

8604d75

Xen ACPI table support_v2

b05c34e

Xen ACPI table support_v2

c3d5993

Xen ACPI tables support_v3

e9625f1

Merge branch 'master' of https://github.com/tianocore/edk2

0897f81

Xen ACPI tables support_v4

5728647

Xen PV support and a simple block driver

f197da1

jljusten closed this Aug 28, 2012

jiaxinwu mentioned this pull request Nov 10, 2023

Fix CP Exception when CET enable #4867

Merged

mdkinney mentioned this pull request Feb 14, 2024

Bug 4683 unit test unexpected exception may be seen as PASS condition #5345

Merged

tlendacky mentioned this pull request Jun 2, 2024

Update AMD SEV-SNP related defines #5708

Merged

3 tasks

kraxel mentioned this pull request Jun 13, 2024

AMD SEV Debugswap #5594

Merged

kraxel mentioned this pull request Aug 21, 2024

Optimize the X2APIC mode enabling logic in MpInitLib #5983

Merged

3 tasks

kraxel mentioned this pull request Sep 2, 2024

OvmfPkg: Use TdHob instead of e820tables to get memory info in TDVF #6114

Merged

3 tasks

kraxel added a commit to kraxel/edk2 that referenced this pull request Sep 10, 2024

submodule: add fix tianocore#1

a019467

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

kraxel mentioned this pull request Sep 10, 2024

update to openssl-3.4.x #6184

Merged

niruiyu mentioned this pull request Oct 10, 2024

Smmcpu platform hook Before MMI Handler #6183

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Xen ACPI tables support in OVMF #1

Xen ACPI tables support in OVMF #1

GavinGuan commented Jul 5, 2012

jljusten commented Aug 28, 2012

Xen ACPI tables support in OVMF #1

Xen ACPI tables support in OVMF #1

Conversation

GavinGuan commented Jul 5, 2012

jljusten commented Aug 28, 2012