Due to needs a tackling the deficiency of the AP API, it's necessary to
ensure that in non-blocking mode previous AP executed command is
finished before starting new one.
To remedy above:
1) execute AcquireSpinLock instead AcquireSpinLockOrFail - this will
ensure time "window" to eliminate potential race condition between
BSP and AP spinLock release in non-blocking mode.
This also will eliminate possibility to start executing new AP
function before last is finished.
2) remove returns EFI_STATUS - EFI_NOT_READY - in new scenario returned
status is not necessary to caller.
Signed-off-by: Damian Nikodem <damian.nikodem@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Krzysztof Rusocki <krzysztof.rusocki@intel.com>
Today's behavior is to enable 5l paging when CPU supports it
(CPUID[7,0].ECX.BIT[16] is set).
The patch changes the behavior to enable 5l paging when two
conditions are both met:
1. CPU supports it;
2. The max physical address bits is bigger than 48.
Because 4-level paging can support to address physical address up to
2^48 - 1, there is no need to enable 5-level paging with max
physical address bits <= 48.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Today's behavior is to always restrict access to non-SMRAM regardless
the value of PcdCpuSmmRestrictedMemoryAccess.
Because RAS components require to access all non-SMRAM memory, the
patch changes the code logic to honor PcdCpuSmmRestrictedMemoryAccess
so that only when the PCD is true, the restriction takes affect and
page table memory is also protected.
Because IA32 build doesn't reference this PCD, such restriction
always takes affect in IA32 build.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The patch changes PiSmmCpu driver to consume PCD
PcdCpuSmmRestrictedMemoryAccess.
Because the behavior controlled by PcdCpuSmmStaticPageTable in
original code is not changed after switching to
PcdCpuSmmRestrictedMemoryAccess.
The functionality is not impacted by this patch.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2040
Supports new logic which test current value before write new value.
Only write new value when current value not same as new value.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reclaim may free page table pages that are required to handle current page
fault. This causes a page leak, and, after sufficent number of specific
page fault+reclaim pairs, we run out of reclaimable pages and hit:
ASSERT (MinAcc != (UINT64)-1);
To remedy, prevent pages essential to handling current page fault:
(1) from being considered as reclaim candidates (first reclaim phase)
(2) from being freed as part of "branch cleanup" (second reclaim phase)
Signed-off-by: Damian Nikodem <damian.nikodem@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Krzysztof Rusocki <krzysztof.rusocki@intel.com>
1. Update CPUStatus to CpuStatus in comments to align comments
with code.
2. Add "OUT" attribute for "ProcedureArguments" parameter in function
header.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2060
Remove the useless ConsoleLogLock spinlock.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
This debug message may be called by BSP and APs. It may
caused ASSERT when APs call this debug code.
In order to avoid system boot assert, Remove this debug
message.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
1.Remove "out" attribute for " Buffer" parameter in function header.
2.Add "out" attribute for " Token" parameter in function header.
3.Update ProcedureArgument to ProcedureArguments.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
The pointer Pml5Entry, returned from call to function
AllocatePageTableMemory, may be null.
So add check for it.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
This reverts commit 30f6148546.
Commit 30f6148546 causes a build failure, when building for IA32:
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c: In function
> 'PerformRemainingTasks':
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c:1440:9: error:
> 'mCpuSmmStaticPageTable' undeclared (first use in this function)
> if (mCpuSmmStaticPageTable) {
"mCpuSmmStaticPageTable" is an X64-only variable. It is defined in
"X64/PageTbl.c", which is not linked into the IA32 binary. We must not
reference the variable in such code that is linked into both IA32 and X64
builds, such as "PiSmmCpuDxeSmm.c".
We have encountered the same challenge at least once in the past:
- https://bugzilla.tianocore.org/show_bug.cgi?id=1593
- commit 37f9fea5b8 ("UefiCpuPkg\CpuSmm: Save & restore CR2 on-demand
paging in SMM", 2019-04-04)
The right approach is to declare a new function in "PiSmmCpuDxeSmm.h", and
to provide two definitions for the function, one in "Ia32/PageTbl.c", and
another in "X64/PageTbl.c". The IA32 implementation should return a
constant value. The X64 implementation should return
"mCpuSmmStaticPageTable". (In the example named above, the functions were
SaveCr2() and RestoreCr2().)
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: push revert immediately, due to build breakage that
would have been easy to catch before submitting the patch]
Commit c60d36b4d1
* UefiCpuPkg/SmmCpu: Block access-out only when static paging is used
updated page fault handler to treat SMM access-out as allowed
address when static paging is not used.
But that commit is not complete because the page table is still
updated in SetUefiMemMapAttributes() for non-SMRAM memory. When SMM
code accesses non-SMRAM memory, page fault is still generated.
This patch skips to update page table for non-SMRAM memory and
page table itself.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1937
Add MM Mp Protocol in PiSmmCpuDxeSmm driver.
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Fixes: 4eee0cc7c
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1946
The patch changes SMM environment to use 5 level paging when CPU
supports it.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
(cherry picked from commit 7365eb2c8c)
This reverts commit 7365eb2c8c.
Commit
7c5010c7f8 MdePkg/BaseLib.h: Update IA32_CR4 structure for 5-level paging
technically breaks the EDKII development process documented in
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process
and Maintainers.txt in EDKII repo root directory.
The voilation is commit 7c5010c7f8 doesn't have a Reviewed-by or
Acked-by from MdePkg maintainers.
In order to revert 7c5010c7f8, 7365eb2c8 needs to revert first otherwise
simply reverting 7c5010c7f8 will cause build break.
Signed-off-by: Ray Ni <ray.ni@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1946
The patch changes SMM environment to use 5 level paging when CPU
supports it.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Per SDM, for IA-32e 4-KByte paging, there are four layers in the page
table structure:
1. PML4
2. Page-Directory-Pointer Table (PDPT)
3. Page-Directory (PD)
4. Page Table (PT)
The patch changes the local variable names and comments to use
"PML4", "PDPT", "PD", "PT" to better align to terms used in SDM.
There is no functionality impact for this change.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1136
CVE: CVE-2018-12182
Customer met system hang-up during serial port loopback test in OS.
It is a corner case happened with one CPU core doing "out dx,al" and
another CPU core(s) doing "rep outs dx,byte ptr [rsi]".
Detailed code flow is as below.
1. Serial port loopback test in OS.
One CPU core: "out dx,al" -> Writing B2h, SMI will happen.
Another CPU core(s): "rep outs dx,byte ptr [rsi]".
2. SMI happens to enter SMM.
"out dx" (SMM_IO_TYPE_OUT_DX) is saved as I/O instruction type in
SMRAM save state for CPU doing "out dx,al".
"rep outs dx" (SMM_IO_TYPE_REP_OUTS) is saved as I/O instruction
type and rsi is save as I/O Memory Address in SMRAM save state for
CPU doing "rep outs dx, byte ptr [rsi]".
NOTE: I/O Memory Address (rsi) is a virtual address mapped by
OS/Virtual Machine.
3. Some SMM code calls EFI_SMM_CPU_PROTOCOL.ReadSaveState() with
EFI_SMM_SAVE_STATE_REGISTER_IO and parse data returned.
For example:
https://github.com/tianocore/edk2/blob/master/QuarkSocPkg/
QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNC/QNCSmmSw.c#L76
4. SmmReadSaveState() is executed to read save state for
EFI_SMM_SAVE_STATE_REGISTER_IO.
- The SmmReadSaveState() function in
"UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c" calls the
SmmCpuFeaturesReadSaveStateRegister() function, from the platform's
SmmCpuFeaturesLib instance.
- If that platform-specific function returns EFI_UNSUPPORTED, then
PiSmmCpuDxeSmm falls back to the common function
ReadSaveStateRegister(), defined in file
"UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c".
Current ReadSaveStateRegister() in
UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c is trying to copy data
from I/O Memory Address for EFI_SMM_SAVE_STATE_IO_TYPE_REP_PREFIX,
PF will happen as SMM page table does not know and cover this
OS/Virtual Machine virtual address.
Same case is for SmmCpuFeaturesReadSaveStateRegister() in platform-
specific SmmCpuFeaturesLib instance if it has similar implementation
to read save state for EFI_SMM_SAVE_STATE_REGISTER_IO with
EFI_SMM_SAVE_STATE_IO_TYPE_REP_PREFIX.
Same case is for "ins", 'outs' and 'rep ins'.
So to fix the problem, this patch updates the code to only support
IN/OUT, but not INS/OUTS/REP INS/REP OUTS for SmmReadSaveState().
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1593
For every SMI occurrence, save and restore CR2 register only when SMM
on-demand paging support is enabled in 64 bit operation mode.
This is not a bug but to have better improvement of code.
Patch5 is updated with separate functions for Save and Restore of CR2
based on review feedback.
Patch6 - Removed Global Cr2 instead used function parameter.
Patch7 - Removed checking Cr2 with 0 as per feedback.
Patch8 and 9 - Aligned with EDK2 Coding style.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Vanguput Narendra K <narendra.k.vanguput@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1521
We scan the SMM code with ROPgadget.
http://shell-storm.org/project/ROPgadget/https://github.com/JonathanSalwan/ROPgadget/tree/master
This tool reports the gadget in SMM driver.
This patch enabled CET ShadowStack for X86 SMM.
If CET is supported, SMM will enable CET ShadowStack.
SMM CET will save the OS CET context at SmmEntry and
restore OS CET context at SmmExit.
Test:
1) test Intel internal platform (x64 only, CET enabled/disabled)
Boot test:
CET supported or not supported CPU
on CET supported platform
CET enabled/disabled
PcdCpuSmmCetEnable enabled/disabled
Single core/Multiple core
PcdCpuSmmStackGuard enabled/disabled
PcdCpuSmmProfileEnable enabled/disabled
PcdCpuSmmStaticPageTable enabled/disabled
CET exception test:
#CF generated with PcdCpuSmmStackGuard enabled/disabled.
Other exception test:
#PF for normal stack overflow
#PF for NX protection
#PF for RO protection
CET env test:
Launch SMM in CET enabled/disabled environment (DXE) - no impact to DXE
The test case can be found at
https://github.com/jyao1/SecurityEx/tree/master/ControlFlowPkg
2) test ovmf (both IA32 and X64 SMM, CET disabled only)
test OvmfIa32/Ovmf3264, with -D SMM_REQUIRE.
qemu-system-x86_64.exe -machine q35,smm=on -smp 4
-serial file:serial.log
-drive if=pflash,format=raw,unit=0,file=OVMF_CODE.fd,readonly=on
-drive if=pflash,format=raw,unit=1,file=OVMF_VARS.fd
QEMU emulator version 3.1.0 (v3.1.0-11736-g7a30e7adb0-dirty)
3) not tested
IA32 CET enabled platform
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yao Jiewen <jiewen.yao@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1091
Previously, when compiling NASM source files, BaseTools did not support
including files outside of the NASM source file directory. As a result, we
duplicated multiple copies of "StuffRsb.inc" files in UefiCpuPkg. Those
INC files contain the common logic to stuff the Return Stack Buffer and
are identical.
After the fix of BZ 1085:
https://bugzilla.tianocore.org/show_bug.cgi?id=1085
The above support was introduced.
Thus, this commit will merge all the StuffRsb.inc files in UefiCpuPkg into
one file. The merged file will be named 'StuffRsbNasm.inc' and be placed
under folder UefiCpuPkg/Include/.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1417
Since BaseLib API AsmLfence() is a x86 arch specific API and should be
avoided using in generic codes, this commit replaces the usage of
AsmLfence() with arch-generic API SpeculationBarrier().
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
In current implementation, core and package level sync uses same semaphores.
Sharing the semaphore may cause wrong execution order.
For example:
1. Feature A has CPU_FEATURE_CORE_BEFORE dependency with Feature B.
2. Feature C has CPU_FEATURE_PACKAGE_AFTER dependency with Feature B.
The expected feature initialization order is A B C:
A ---- (Core Depends) ----> B ---- (Package Depends) ----> C
For a CPU has 1 package, 2 cores and 4 threads. The feature initialization
order may like below:
Thread#1 Thread#2 Thread#3 Thread#4
[A.Init] [A.Init] [A.Init]
Release(S1, S2) Release(S1, S2) Release(S3, S4)
Wait(S1) * 2 Wait(S2) * 2 <------------------------------- Core sync
[B.Init] [B.Init]
Release (S1,S2,S3,S4)
Wait (S1) * 4 <----------------------------------------------------- Package sync
Wait(S4 * 2) <- Core sync
[B.Init]
In above case, for thread#4, when it syncs in core level, Wait(S4) * 2 isn't
blocked and [B.Init] runs. But [A.Init] hasn't run in thread#3. It's wrong!
Thread#4 should execute [B.Init] after thread#3 executes [A.Init] because B
core level depends on A.
The reason of the wrong execution order is that S4 is released in thread#1
by calling Release (S1, S2, S3, S4) and in thread #4 by calling
Release (S3, S4).
To fix this issue, core level sync and package level sync should use separate
semaphores.
In above example, the S4 released in Release (S1, S2, S3, S4) should not be the
same semaphore as that in Release (S3, S4).
Related BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1311
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
When static paging is disabled, page table for below 4GB is created
and page table for above 4GB is created dynamically in page fault
handler.
Today's implementation only allow SMM access-out to below types of
memory address no matter static paging is enabled or not:
1. Reserved, run time and ACPI NVS type
2. MMIO
But certain platform feature like RAS may need to access other types
of memory from SMM. Today's code blocks these platforms.
This patch simplifies the policy to only block when static paging
is used so that the static paging can be disabled in these platforms
to meet their SMM access-out need.
Setting PcdCpuSmmStaticPageTable to FALSE can disable the static
paging.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Index is initialized to MAX_UINT16 as default failure value, which
is what the ASSERT is supposed to test for. The ASSERT condition
however can never return FALSE for INT16 != int, as due to
Integer Promotion[1], Index is converted to int, which can never
result in -1.
Furthermore, Index is used as a for loop index variable inbetween its
initialization and the ASSERT, so the value is unconditionally
overwritten too.
Fix the ASSERT check to compare Index to its upper boundary, which it
will be equal to if the loop was not broken out of on success.
[1] ISO/IEC 9899:2011, 6.5.9.4
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Code initialized in function can't be correctly detected by build tool.
Add code to clearly initialize the local variable before use it.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Remove extra white space at the end of line.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
V4 changes:
1. Serial console log for different threads when program register table.
2. Check the AcpiCpuData before use it to avoid potential ASSERT.
V3 changes:
1. Use global variable instead of internal function to return string for register type
and dependence type.
2. Add comments for some complicated logic.
V1 changes:
Because this driver needs to set MSRs saved in normal boot phase, sync
semaphore logic from RegisterCpuFeaturesLib code which used for normal boot phase.
Detail see below change for RegisterCpuFeaturesLib:
UefiCpuPkg/RegisterCpuFeaturesLib: Add logic to support semaphore type.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1194
Speculative execution is used by processor to avoid having to wait for
data to arrive from memory, or for previous operations to finish, the
processor may speculate as to what will be executed.
If the speculation is incorrect, the speculatively executed instructions
might leave hints such as which memory locations have been brought into
cache. Malicious actors can use the bounds check bypass method (code
gadgets with controlled external inputs) to infer data values that have
been used in speculative operations to reveal secrets which should not
otherwise be accessed.
It is possible for SMI handler(s) to call EFI_SMM_CPU_PROTOCOL service
ReadSaveState() and use the content in the 'CommBuffer' (controlled
external inputs) as the 'CpuIndex'. So this commit will insert AsmLfence
API to mitigate the bounds check bypass issue within SmmReadSaveState().
For SmmReadSaveState():
The 'CpuIndex' will be passed into function ReadSaveStateRegister(). And
then in to ReadSaveStateRegisterByIndex().
With the call:
ReadSaveStateRegisterByIndex (
CpuIndex,
SMM_SAVE_STATE_REGISTER_IOMISC_INDEX,
sizeof(IoMisc.Uint32),
&IoMisc.Uint32
);
The 'IoMisc' can be a cross boundary access during speculative execution.
Later, 'IoMisc' is used as the index to access buffers 'mSmmCpuIoWidth'
and 'mSmmCpuIoType'. One can observe which part of the content within
those buffers was brought into cache to possibly reveal the value of
'IoMisc'.
Hence, this commit adds a AsmLfence() after the check of 'CpuIndex'
within function SmmReadSaveState() to prevent the speculative execution.
A more detailed explanation of the purpose of commit is under the
'Bounds check bypass mitigation' section of the below link:
https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation
And the document at:
https://software.intel.com/security-software-guidance/api-app/sites/default/files/337879-analyzing-potential-bounds-Check-bypass-vulnerabilities.pdf
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.
After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.
This patch updates PiSmmCpuDxeSmm to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.
Cc: Younas khan <pmdyounaskhan786@gmail.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1191
Before commit e21e355e2c, jmp _SmiHandler
is commented. And below code, ASM_PFX(CpuSmmDebugEntry) is moved into rax,
then call it. But, this code doesn't work in XCODE5 tool chain. Because XCODE5
doesn't generated the absolute address in the EFI image. So, rax stores the
relative address. Once this logic is moved to another place, it will not work.
; jmp _SmiHandler ; instruction is not needed
...
mov rax, ASM_PFX(CpuSmmDebugEntry)
call rax
Commit e21e355e2c is to support XCODE5.
One tricky way is selected to fix it. Although SmiEntry logic is copied to
another place and run, but here jmp _SmiHandler is enabled to jmp the original
code place, then call ASM_PFX(CpuSmmDebugEntry) with the relative address.
mov rax, strict qword 0 ; mov rax, _SmiHandler
_SmiHandlerAbsAddr:
jmp rax
...
call ASM_PFX(CpuSmmDebugEntry)
Now, BZ 1191 raises the issue that SmiHandler should run in the copied address,
can't run in the common address. So, jmp _SmiHandler is required to be removed,
the code is kept to run in copied address. And, the relative address is
requried to be fixed up to the absolute address. The necessary changes should
not affect the behavior of platforms that already consume PiSmmCpuDxeSmm.
OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain has been verified.
...
mov rax, strict qword 0 ; call ASM_PFX(CpuSmmDebugEntry)
CpuSmmDebugEntryAbsAddr:
call rax
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Some redundant library classes Ppis and GUIDs
have been removed in inf, .c and .h files.
v2:
1.Remove ReadOnlyVariable2.h in S3Resume.c which should be
deleted in last version in which gEfiPeiReadOnlyVariable2PpiGuid
was removed.
2.Remove the library class BaseLib in CpuPageTable.c
which is included elsewhere.
3.Add library classes in SecCore.inf which are removed
at last version.
They are DebugAgentLib and CpuExceptionHandlerLib.
4.Add two Ppis in SecCore.inf which are removed
at last version.
They are gEfiSecPlatformInformationPpiGuid and
gEfiSecPlatformInformation2PpiGuid.
https://bugzilla.tianocore.org/show_bug.cgi?id=1043https://bugzilla.tianocore.org/show_bug.cgi?id=1013https://bugzilla.tianocore.org/show_bug.cgi?id=1032https://bugzilla.tianocore.org/show_bug.cgi?id=1016
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei <shenglei.zhang@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1165
InitSmmS3Cr3 () will update SmmS3ResumeState so moving the calling of
it into else block to keep the logic consistency.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1165
HOB gEfiAcpiVariableGuid is a must have data for S3 resume if
PcdAcpiS3Enable is set to TRUE. Current code in CpuS3.c doesn't
embody this strong binding between them. An error message and
CpuDeadLoop are added in this patch to warn platform developer
about it.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Since SMM profile feature has already implemented non-stop mode if #PF
occurred, this patch just makes use of the existing implementation to
accommodate heap guard and NULL pointer detection feature.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1093
Return Stack Buffer (RSB) is used to predict the target of RET
instructions. When the RSB underflows, some processors may fall back to
using branch predictors. This might impact software using the retpoline
mitigation strategy on those processors.
This commit will add RSB stuffing logic before returning from SMM (the RSM
instruction) to avoid interfering with non-SMM usage of the retpoline
technique.
After the stuffing, RSB entries will contain a trap like:
@SpecTrap:
pause
lfence
jmp @SpecTrap
A more detailed explanation of the purpose of commit is under the
'Branch target injection mitigation' section of the below link:
https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation
Please note that this commit requires further actions (BZ 1091) to remove
the duplicated 'StuffRsb.inc' files and merge them into one under a
UefiCpuPkg package-level directory (such as UefiCpuPkg/Include/).
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1091
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Current implementation will copy GDT/IDT at SmmReadyToLock point
from ACPI NVS memory to Smram. Later at S3 resume phase, it restore
the memory saved in Smram to ACPI NVS. It can directly use GDT/IDT
saved in Smram instead of restore the original ACPI NVS memory.
This patch do this change.
Test Done:
Do the OS boot and S3 resume test.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Within function GetUefiMemoryAttributesTable(), add a check to avoid
possible null pointer dereference.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
It treats the UEFI runtime page with EFI_MEMORY_RO attribute as
invalid SMM communication buffer.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
It treats GCD untested memory as invalid SMM
communication buffer.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
1. Do not use tab characters
2. No trailing white space in one line
3. All files must end with CRLF
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao <liming.gao@intel.com>
NASM introduced FXSAVE / FXRSTOR support in commit 900fa5b26b8f ("NASM
0.98p3-hpa", 2002-04-30), which commit stands for the nasm-0.98p3-hpa
release.
NASM introduced FXSAVE64 / FXRSTOR64 support in commit 3a014348ca15
("insns: add FXSAVE64/FXRSTOR64, drop np prefix", 2010-07-07), which was
part of the "nasm-2.09" release.
Edk2 requires nasm-2.10 or later for use with the GCC toolchain family,
and nasm-2.12.01 or later for use with all other toolchain families.
Replace the binary encoding of the FXSAVE(64)/FXRSTOR(64) instructions
with mnemonics.
I verified that the "Ia32/SmiException.obj", "X64/SmiEntry.obj" and
"X64/SmiException.obj" files are rebuilt after this patch, without any
change in content.
This patch removes the last instructions encoded with DBs from
PiSmmCpuDxeSmm.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=866
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Damian Nikodem
Ray Ni
Dong, Eric
Shenglei Zhang
Laszlo Ersek
Star Zeng
Michael D Kinney
Vanguput, Narendra K
Jiewen Yao
Hao Wu
Marvin H?user
Liming Gao
Jian J Wang