Support PAE paging for PageTableParse API in CpuPageTableLib.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
The PEI instance of the CpuExceptionHandlerLib didn't implement the
RegisterCpuInterruptHandler() API. This patch adds the missing API.
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4083
In CPU relaxed mode, it doesn't reset the value of
mSmmMpSyncData->AllApArrivedWithException when BSP exit smm mode.
So this patch will reset this variable.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhihao Li <zhihao.li@intel.com>
Reviewed-by: Abner Chang <abner.chang@amd.com>
This commit is a code optimization to allow bigger seperate stack size in
ArchSetupExceptionStack. In previous code logic, CPU_STACK_ALIGNMENT bytes
will be wasted if StackTop is already CPU_STACK_ALIGNMENT aligned.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Reviewed-by: Abner Chang <abner.chang@amd.com>
Parallelly run the function to SeparateExceptionStacks for all CPUs and
allocate buffers together for better performance.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
To remove the dependency of CPU register, 4/8 byte at the top of the
stack is occupied for CpuMpData. BIST information is also taken care
here. This modification is only for PEI phase, since in DXE phase
CpuMpData is accessed via global variable.
Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
The API of InitializeSeparateExceptionStacks is just changed before, and
makes the struct CPU_EXCEPTION_INIT_DATA an internal definition.
Furthermore, we can even remove the struct to make core simpler.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
When switch bsp, old bsp and new bsp put CR0/CR4 into stack, and put IDT
and GDT register into a structure. After they exchange their stack, they
restore these registers. This logic is now implemented by assembly code.
This patch aims to reuse (Save/Restore)VolatileRegisters function to
replace such assembly code for better code readability.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3962
Two SMM variables (mSmrrSupported & mSmmFeatureControlSupported) are global
variables, they control whether the SMRR and SMM Feature Control MSR will
be restored respectively.
To avoid the TOCTOU, add PCD to control SMRR & SmmFeatureControl enable.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
Currently, when waking up AP, IDT table of AP will be set in 16 bit code,
and assume the IDT table base is 32 bit. However, the IDT table is created
by BSP. Issue will happen if the BSP allocates memory above 4G for BSP's
IDT table. Moreover, even the IDT table location is below 4G, the handler
function inside the IDT table is 64 bit, and it won't take effect until
CPU transfers to 64 bit long mode. There is no benefit to set IDT table in
such an early phase.
To avoid such issue, this patch moves the LIDT instruction into 64 bit
code.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Add host based unit tests for the CpuPageTableLib services.
Unit test focuses on PageTableMap function, containing two kinds of test
cases: manual test case and random test case.
Manual test case creates some corner case to test function PageTableMap.
Random test case generates multiple random memory entries (with random
attribute) as the input of function PageTableMap to get the output
pagetable. Output pagetable will be validated and be parsed to get output
memory entries, and then the input and output memory entries will be
compared to verify the functionality.
The unit test is not perfect yet. There are options for random test, and
some of them control the test coverage, and some option are not ready.
Will enhance in the future.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
This reverts commit 2812668bfc for tag202208.
This feature will be merged after stable tag 202208 is created.
Signed-off-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
Add host based unit tests for the CpuPageTableLib services.
Unit test focuses on PageTableMap function, containing two kinds of test
cases: manual test case and random test case.
Manual test case creates some corner case to test function PageTableMap.
Random test case generates multiple random memory entries (with random
attribute) as the input of function PageTableMap to get the output
pagetable. Output pagetable will be validated and be parsed to get output
memory entries, and then the input and output memory entries will be
compared to verify the functionality.
The unit test is not perfect yet. There are options for random test, and
some of them control the test coverage, and some option are not ready.
Will enhance in the future.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
This patch is code refactoring and doesn't change any functionality.
Remove mInternalCr3 in PiSmmCpuDxe pagetable related code. In previous
code, mInternalCr3 is used to pass address of page table which is
different from Cr3 register in different level of SetMemoryAttributes
function. Now remove it and pass the page table base address from the
root function parameter to simplify the code logic.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
This patch is code refactoring and doesn't change any functionality.
Add a new mIsShadowStack flag to identify whether current memory is
shadow stack. Previous smm code logic regards a RO range as shadow
stack and set the dirty bit in corresponding page table entry if
mInternalCr3 is not 0, which may be confusing.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
The change doesn't change functionality behavior.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
With following paging structure to map
[2M-4K, 2M] as P = 1, RW = 0,
[2M, 4M] as P = 1, RW = 1:
PML4[0] -> PDPTE[0] -> PDE[0](RW = 0) -> PTE[255](P = 0, RW = 0)
-> PDE[1](RW = 1)
When a new request to map [2M-4K, 2M+4K] as P = 1, RW = 1,
CpuPageTableMap() wrongly requests 4K buffer size for the new mapping
request.
But in fact, for [2M-4K, 2M] request, PTE[255] can be changed in place,
for [2M, 2M+4K], no change is needed because PDE[1].RW = 1 already.
The change fixes the bug.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
With the following paging structure that maps [0, 2G] with ReadWrite
bit set.
PML4[0] --> PDPTE[0] --> PDE[0-255]
\-> PDPTE[1] --> PDE[0-255]
If ReadWrite bit is cleared in PML4[0] and PageTableMap() is called
to change [0, 2M] as read-only, today's logic unnecessarily changes
the paging structure in 2 aspects:
1. When setting PageTableBaseAddress in the entry, the code clears
all attributes.
2. Even the ReadWrite bit in parent entry is not set, the code clears
the ReadWrite bit in the leaf entry.
First change is wrong. It should not change other attributes when
setting the PA.
Second change is unnecessary. Because the parent entry already
declares the whole region as read-only, there is no need to clear
ReadWrite bit in the leaf entry again.
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
With the following paging structure that maps [0, 2G] with ReadWrite
bit set.
PML4[0] --> PDPTE[0] --> PDE[0-255]
\-> PDPTE[1] --> PDE[0-255]
If ReadWrite bit is cleared in PML4[0] and PageTableMap() is called
to change [0, 2M] as writable, today's logic doesn't inherit the
parent entry's attributes when determining the child entry's
attributes. It just sets the PDPTE[0].PDE[0].ReadWrite bit.
But since the PML4[0].ReadWrite is 0, [0, 2M] is still read-only.
The change fixes the bug.
If the inheritable attributes in ParentPagingEntry conflicts with the
requested attributes, let the child entries take the parent attributes
and loosen the attribute in the parent entry.
E.g.: when PDPTE[0].ReadWrite = 0 but caller wants to map [0-2MB as
ReadWrite = 1 (PDE[0].ReadWrite = 1), we need to change
PDPTE[0].ReadWrite = 1 and let all PDE[0-255].ReadWrite = 0 first.
Then change PDE[0].ReadWrite = 1.
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Today's logic wrongly treats the non-leaf entry as leaf entry and
updates its paging attributes.
The patch fixes the bug to only update paging attributes for
non-present entries or leaf entries.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
When PageTableMap() is called to create non 1:1 mapping
such as [0, 1G) to [8K, 1G+8K), it should split the page entry to the
4K page level, but old logic has a bug that it just uses 1G page
entry.
The patch fixes the bug.
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
The patch replaces
LinearAddress + Offset == RegionStart
with
((LinearAddress + Offset) & RegionMask) == 0
The replace should not cause any behavior change.
Because:
1. In first loop of while when LinearAddress + Offset == RegionStart,
because the lower "BitStart" bits of RegionStart are all-zero,
all lower "BitStart" bits of (LinearAddress + Offset) are all-zero.
Because all lower "BitStart" bits of RegionMask is all-one and
bits are all-zero, ((LinearAddress + Offset) & RegionMask) == 0.
2. In following loops of the while, even RegionStart is increased
by RegionLength, the lower "BitStart" bits are still all-zero.
So the two expressions still semantically equal to each other.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
When LinearAddress or Length is not aligned on 4KB, PageTableMap()
should return Invalid Parameter.
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
The lib includes two APIs:
* PageTableMap
It creates/updates mapping from LA to PA.
The implementation only supports paging structures used in 64bit
mode now. PAE paging structure support will be added in future.
* PageTableParse
It parses the page table and returns the mapping relations in an
array of IA32_MAP_ENTRY.
It passed some stress tests. These test code will be upstreamed in
other patches following edk2 Unit Test framework.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
CPU_EXCEPTION_INIT_DATA is now an internal implementation of
CpuExceptionHandlerLib. Union can be removed since Ia32 and X64 have the
same definition. Also, two fields (Revision and InitDefaultHandlers)are
useless, can be removed.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Since the API InitializeSeparateExceptionStacks is simplified and does't
use the struct CPU_EXCEPTION_INIT_DATA, CPU_EXCEPTION_INIT_DATA become
a inner implementation of CpuExcetionHandlerLib.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Hide the Exception implementation details in CpuExcetionHandlerLib and
caller only need to provide buffer
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
Currently, "push byte %[Vector]" causes nasm warning when Vector is larger
than 0x7F. This is because push accepts a signed value, and byte means
signed int8. Maximum signed int8 is 0x7F.
When Vector is larger the 0x7F, for example, when Vector is 255, byte 255
turns to -1, and causes the warning "signed byte value exceeds".
To avoid such warning, use dword instead of byte, this will increase 3 bytes
for each IdtVector.
For IA32, the size of IdtVector will increase from 10 bytes to 13 bytes.
For X64, the size of IdtVector will increase from 15 bytes to 18 bytes.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3957
The reserved IDT table size in SecCore is too small for X64. Changed the type
of IdtTable in SEC_IDT_TABLE from UINT64 to IA32_IDT_GATE_DESCRIPTOR to have
sufficient size reserved in IdtTable for X64. dff
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Ted Kuo <ted.kuo@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Add debug messages to make it easier to verify PlatformSecLib
is passing the data properly.
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
A memory range can be submitted for attribute changes which is large
enough to not require a page split during the attribute update. Consider
the following scenario:
1. An attribute update removed the RW attribute on a range large enough
to not require a page split.
2. Later, an attributes update is called to re-add the RW attribute for
a subsection of that larger page which requires a split
3. The attribute update logic performs a page split, so now the parent
and child pages have matching attributes
4. Then, the attribute update logic changes the child page to have the
RW attribute.
5. The child page would then correctly have the RW attribute added but
the parent page would still have the RW attribute removed which will
cause an improper access violation.
The page being split should have loose attributes to accommodate the
above case. The split page should always have the attributes set so
the lowest level page frame determines the access rights as detailed
in 4.10.2.2 of the Intel 64 and IA-32 Architectures Software
Developer Manual. Setting the User/Supervisor attribute shouldn't
be necessary.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Taylor Beebe <t@taylorbeebe.com>
The AP vector consists of 2 parts:
1. the initial 16-bit code that should be under 1MB and page aligned.
2. the 32-bit/64-bit code that can be anywhere in the memory with any
alignment.
The need of part #2 is because the memory under 1MB is temporary
"stolen" for use and will "give" back after all AP wake up. The range
of memory is not marked as code page in page table. CPU may trigger
exception as soon as NX is enabled.
The part #2 memory allocation can be done in the MpInitLibInitialize.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Today's implementation allocates below 1MB memory for the 16bit, 32bit
and 64bit code.
But it's not necessary since now the 32bit and 64bit code run at high
memory no matter in PEI and DXE phase.
The patch simplifies the logic to remove the code that handles the
case when WakeupBufferHigh is 0.
It also reduce the memory foot print under 1MB by allocating
memory for 16bit code only.
MP_CPU_EXCHANGE_INFO is still under 1MB which is immediate
after the 16bit code.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
The patch does several simplifications:
1. Treat SwitchToRealProc as part of RendezvousFunnelProc.
So the common logic in MpLib.c doesn't need to be aware of
SwitchToRealProc.
As a result, SwitchToRealSize/Offset are removed from
MP_ASSEMBLY_ADDRESS_MAP.
2. Move SwitchToRealProc to AmdSev.nasm.
All other assembly code in AmdSev.nasm is called through
OneTimeCall.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Tested-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Michael Roth <michael.roth@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
global in NASM file is used for symbols that are
referenced in C files.
Remove unneeded global keyword in NASM file.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Today's implementation assumes PEI phase runs at 32bit so
the execution-disable feature is not applicable.
It's not always TRUE.
The patch allocates 32bit&64bit code buffer for PEI phase as well.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Today InitializeCpuExceptionHandlersEx is called from three modules:
1. DxeCore (links to DxeCpuExceptionHandlerLib)
DxeCore expects it initializes the IDT entries as well as
assigning separate stacks for #DF and #PF.
2. CpuMpPei (links to PeiCpuExceptionHandlerLib)
and CpuDxe (links to DxeCpuExceptionHandlerLib)
It's called for each thread for only assigning separate stacks for
#DF and #PF. The IDT entries initialization is skipped because
caller sets InitData->X64.InitDefaultHandlers to FALSE.
Additionally, SecPeiCpuExceptionHandlerLib, SmmCpuExceptionHandlerLib
also implement such API and the behavior of the API is simply to initialize
IDT entries only.
Because it mixes the IDT entries initialization and separate stacks
assignment for certain exception handlers together, in order to know
whether the function call only initializes IDT entries, or assigns stacks,
we need to check:
1. value of InitData->X64.InitDefaultHandlers
2. library instance
This patch cleans up the code to separate the stack assignment to a new API:
InitializeSeparateExceptionStacks().
Only when caller calls the new API, the separate stacks are assigned.
With this change, the SecPei and Smm instance can return unsupported which
gives caller a very clear status.
The old API InitializeCpuExceptionHandlersEx() is removed in this patch.
Because no platform module is consuming the old API, the impact is none.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
InitializeCpuExceptionHandlers() expects caller allocates IDT while
InitializeCpuInterruptHandlers() allocates 256 IDT entries itself.
InitializeCpuExceptionHandlers() fills max 32 IDT entries allocated
by caller. If caller allocates 10 entries, the API just fills 10 IDT
entries.
The inconsistency between the two APIs makes code hard to
unerstand and hard to share.
Because there is only one caller (CpuDxe) for
InitializeCpuInterruptHandler(), this patch updates CpuDxe driver
to allocates 256 IDT entries then call
InitializeCpuExceptionHandlers().
This is also a backward compatible change.
With this change, InitializeCpuInterruptHandlers() is removed
completely.
And InitializeCpuExceptionHandlers() fills max 32 entries for PEI
and SMM instance, max 256 entries for DXE instance.
Such behavior matches to the original one.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Additionally removed two useless global variables:
"SPIN_LOCK mDisplayMessageSpinLock" from SMM instance.
"UINTN mEnabledInterruptNum" from DXE instance.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Today the DXE instance allocates code page and then copies the IDT
vectors to the allocated code page. Then it fixes up the vector number
in the IDT vector.
But if we update the NASM file to generate 256 IDT vectors, there is
no need to do the copy and fix-up.
A side effect is 4096 bytes (HOOKAFTER_STUB_SIZE * 256) is used for
256 IDT vectors while 32 IDT vectors only require 512 bytes without
this change, in following library instances:
1. 32bit SecPeiCpuExceptionHandlerLib and PeiCpuExceptionHandlerLib
2. 64bit PeiCpuExceptionHandlerLib
But considering the code logic simplification, 3.5K extra space is
not a big deal.
If 3.5K is too much, we can enhance the code further to generate 32
vectors for above mentioned library instances.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Acked-by: Eric Dong <eric.dong@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3918
This reverts commit 88da06ca76.
This commit triggers the ASSERT in Non-Td guest.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
Tested-by: Tom Lendacky <thomas.lendacky@amd.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF? https://bugzilla.tianocore.org/show_bug.cgi?id=3912
UefiCpuPkg define a new Protocol with the new services
SmmWaitForAllProcessor(), which can be used by SMI handler
to optionally wait for other APs to complete SMM rendezvous in
relaxed AP mode.
VariableSmm and VariableStandaloneMM driver in MdeModulePkg need
to use this services but MdeModulePkg can't depend on UefiCpuPkg.
Thus, the solution is moving SmmCpuRendezvouslib.h from UefiCpuPkg
to MdePkg and creating SmmCpuRendezvousLib NullLib version
implementation in MdePkg as dependency for the pkg that can't
depend on UefiCpuPkg.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Signed-off-by: Zhihao Li <zhihao.li@intel.com>
Acked-by: Liming Gao <gaoliming@byosoft.com.cn>
There are two libraries: MdePkg/CpuLib and UefiCpuPkg/UefiCpuLib and
UefiCpuPkg/UefiCpuLib will be merged to MdePkg/CpuLib. To avoid build
failure, add CpuLib dependency to all modules that depend on UefiCpuLib.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Yu Pu <yu.pu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3711
Per SDM, changing the mode of APIC timer (from one-shot to periodic or
vice versa) by writing to the timer LVT entry does not start the timer.
To start the timer, it is necessary to write to the initial-count
register.
If initial-count is wrote before mode change, it's possible that timer
expired before the mode change. Thus failing the periodic mode.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
MMIO region in Tdx guest is set with PcdTdxSharedBitMask in TdxDxe's
entry point. In SEV guest the page table entries is set with
PcdPteMemoryEncryptionAddressOrMask when creating 1:1 identity table.
So the AddressEncMask in GetPageTableEntry (@CpuPageTable.c) is either
PcdPteMemoryEncryptionAddressOrMask (in SEV guest), or
PcdTdxSharedBitMask (in TDX guest), or all-0 (in Legacy guest).
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
In TDVF BSP and APs are simplified. BSP is the vCPU-0, while the others
are treated as APs.
So MP intialization is rather simple. ApWorker is not supported, BSP is
always the working processor, while the APs are just in a
wait-for-precedure state.
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
MSR is accessed in BaseXApicX2ApicLib. In TDX some MSRs are accessed
directly from/to CPU. Some should be accessed via explicit requests
from the host VMM using TDCALL(TDG.VP.VMCALL). This is done by the
help of TdxLib.
Please refer to [TDX] Section 18.1
TDX: https://software.intel.com/content/dam/develop/external/us/en/
documents/tdx-module-1.0-public-spec-v0.931.pdf
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
Add base support to handle #VE exceptions. Update the common exception
handlers to invoke the VmTdExitHandleVe () function of the VmgExitLib
library when a #VE is encountered. A non-zero return code will propagate
to the targeted exception handler.
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
VmgExitLib performs the necessary processing to handle a #VC exception.
VmgExitLibNull is a NULL instance of VmgExitLib which provides a
default limited interface. In this commit VmgExitLibNull is extended to
handle a #VE exception with a default limited interface. A full feature
version of #VE handler will be created later.
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3870
The new algorithm searches FFS3 GUID first and then FFS2 GUID at
every 4KB address in the top 16MB just below 4GB.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Reviewed-by: Min Xu <min.m.xu@intel.com>
Signed-off-by: Ted Kuo <ted.kuo@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3862
The new algorithm searches BFV address with FFS3 GUID first.
If not found, it will search BFV address with FFS2 GUID.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Ted Kuo <ted.kuo@intel.com>
To keep the declaration same with definition, remove the last optional
in declaration of WakeUpAP.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Wenyi Xie <xiewenyi2@huawei.com>
REF? https://bugzilla.tianocore.org/show_bug.cgi?id=3815
This patch define a new Protocol with the new services
SmmWaitForAllProcessor(), which can be used by SMI handler
to optionally wait for other APs to complete SMM rendezvous in
relaxed AP mode.
A new library SmmCpuRendezvousLib is provided to abstract the service
into library API to simple SMI handler code.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Zhihao Li <zhihao.li@intel.com>
Signed-off-by: Zhihao Li <zhihao.li@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3790
Replace Opcode with the corresponding instructions.
The code changes have been verified with CompareBuild.py tool, which
can be used to compare the results of two different EDK II builds to
determine if they generate the same binaries.
(tool link: https://github.com/mdkinney/edk2/tree/sandbox/CompareBuild)
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3683
TCG specification says BIOS should extend measurement of microcode to TPM.
However, reference BIOS is not doing this. BIOS shall extend measurement of
microcode to TPM.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min M Xu <min.m.xu@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Signed-off-by: Longlong Yang <longlong.yang@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
Use the SEV-SNP AP Creation NAE event to create and launch APs under
SEV-SNP. This capability will be advertised in the SEV Hypervisor
Feature Support PCD (PcdSevEsHypervisorFeatures).
Cc: Michael Roth <michael.roth@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
During AP bringup, just after switching to long mode, APs will do some
cpuid calls to verify that the extended topology leaf (0xB) is available
so they can fetch their x2 APIC IDs from it. In the case of SEV-ES,
these cpuid instructions must be handled by direct use of the GHCB MSR
protocol to fetch the values from the hypervisor, since a #VC handler
is not yet available due to the AP's stack not being set up yet.
For SEV-SNP, rather than relying on the GHCB MSR protocol, it is
expected that these values would be obtained from the SEV-SNP CPUID
table instead. The actual x2 APIC ID (and 8-bit APIC IDs) would still
be fetched from hypervisor using the GHCB MSR protocol however, so
introducing support for the SEV-SNP CPUID table in that part of the AP
bring-up code would only be to handle the checks/validation of the
extended topology leaf.
Rather than introducing all the added complexity needed to handle these
checks via the CPUID table, instead let the BSP do the check in advance,
since it can make use of the #VC handler to avoid the need to scan the
SNP CPUID table directly, and add a flag in ExchangeInfo to communicate
the result of this check to APs.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Suggested-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
An SEV-SNP guest requires that the physical address of the GHCB must
be registered with the hypervisor before using it. See the GHCB
specification section 2.3.2 for more details.
Cc: Michael Roth <michael.roth@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Ray Ni <ray.ni@Intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
Version 2 of the GHCB specification added a new VMGEXIT that the guest
could use for querying the hypervisor features. One of the immediate
users for it will be an AP creation code. When SEV-SNP is enabled, the
guest can use the newly added AP_CREATE VMGEXIT to create the APs.
The MpInitLib will check the hypervisor feature, and if AP_CREATE is
available, it will use it.
See GHCB spec version 2 for more details on the VMGEXIT.
Cc: Michael Roth <michael.roth@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Ray Ni <ray.ni@Intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
Previous commit introduced a generic confidential computing PCD that can
determine whether AMD SEV-ES is enabled. Update the MpInitLib to drop the
PcdSevEsIsEnabled in favor of PcdConfidentialComputingAttr.
Cc: Michael Roth <michael.roth@amd.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Suggested-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
Move all the SEV specific function in AmdSev.c.
No functional change intended.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Michael Roth <michael.roth@amd.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Suggested-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3737
Apply uncrustify changes to .c/.h files in the UefiCpuPkg package
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3767
Update use of DEBUG_CODE(Expression) if Expression is a complex code
block with if/while/for/case statements that use {}.
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3760
Update all use of ', OPTIONAL' to ' OPTIONAL,' for function params.
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3739
Update all use of EFI_D_* defines in DEBUG() macros to DEBUG_* defines.
Cc: Andrew Fish <afish@apple.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
When CET shadow stack feature is enabled, it needs to use IST for the
exceptions, and uses interrupt shadow stack for the stack switch.
Shadow stack should be 32 bytes aligned.
Check IST field, when clear shadow stack token busy bit when using retf.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3728
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3698
Lots of code relies on CPU Family/Model/Stepping for different logics.
The change adds two APIs for such needs.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
When using UT_ASSERT_EQUAL() on a pointer value, it must be
cast to UINTN. This follows the samples provided with the
UnitTestFrameworkPkg.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3634
The memory allocated through "PeiAllocatePool" is located in HOB, and
in DXE phase, the HOB will be migrated to a different location.
After the migration, the data stored in the HOB stays the same, but the
address of pointer to the memory(such as the pointers in ACPI_CPU_DATA
structure) changes, which may cause "PiSmmCpuDxeSmm" driver can't find
the memory(the pointers in ACPI_CPU_DATA structure) that allocated in
"PeiRegisterCpuFeaturesLib", so use "PeiAllocatePages" to allocate
memory instead.
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3492
Currently SecCore.inf having the resetvector code under IA32. if the
user wants to use both SecCore and UefiCpuPkg ResetVector it's not
possible, since SecCore and ResetVector(VTF0.INF/ResetVector.inf)
are sharing the same GUID which is BFV. to overcome this issue we can
create the Duplicate version of the SecCore.inf as SecCoreNative.inf
which contains pure SecCore Native functionality without resetvector.
SecCoreNative.inf should have the Unique GUID so that it can be used
along with UefiCpuPkg ResetVector in there implementation.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Cc: Digant H Solanki <digant.h.solanki@intel.com>
Cc: Sangeetha V <sangeetha.v@intel.com>
Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3473
X64 Reset Vector Code can access the memory range till 4GB using the
Linear-Address Translation to a 2-MByte Page, when user wants to use
more than 4G using 2M Page it will leads to use more number of Page
table entries. using the 1-GByte Page table user can use more than
4G Memory by reducing the page table entries using 1-GByte Page,
this patch attached can access memory range till 512GByte via Linear-
Address Translation to a 1-GByte Page.
Build Tool: if the nasm is not found it will throw Build errors like
FileNotFoundError: [WinError 2]The system cannot find the file specified
run the command wil try except block to get meaningful error message
Test Result: Tested in both Simulation environment and Hardware
both works fine without any issues.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Cc: Sangeetha V <sangeetha.v@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Sahil Dureja <sahil.dureja@intel.com>
Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3621
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3631
Current CPU feature initialization design:
During normal boot, CpuFeaturesPei module (inside FSP) initializes the
CPU features. During S3 boot, CpuFeaturesPei module does nothing, and
CpuSmm driver (in SMRAM) initializes CPU features instead.
This code change prevents CpuSmm driver from re-initializing CPU
features during S3 resume if CpuFeaturesPei module has done the same
initialization.
In addition, EDK2 contains DxeIpl PEIM that calls S3RestoreConfig2 PPI
during S3 boot and this PPI eventually calls CpuSmm driver (in SMRAM) to
initialize the CPU features, so "EDK2 + FSP" does not have the CPU
feature initialization issue during S3 boot. But "coreboot" does not
contain DxeIpl PEIM and the issue appears, unless
"PcdCpuFeaturesInitOnS3Resume" is set to TRUE.
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3621
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3631
Refactor initialization of CPU features during S3 resume.
In addition, the macro ACPI_CPU_DATA_STRUCTURE_UPDATE is used to fix
incompatibility issue caused by ACPI_CPU_DATA structure update. It will
be removed after all the platform code uses new ACPI_CPU_DATA structure.
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3506
Before executing the nasm command, added print statement to know what
commands are executing.
before printing the output file need check the status of command which
is executed. if the status is 0 then only print the output file name.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Cc: Sangeetha V <sangeetha.v@intel.com>
Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3506
Build Scrips for Reset Vector currently based on Python 2
which is already EOL, needs to modify the build script based on
Python 3
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Cc: Sangeetha V <sangeetha.v@intel.com>
Signed-off-by: Ashraf Ali S <ashraf.ali.s@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2956
In functions ReadSaveStateRegisterByIndex and WriteSaveStateRegister:
* check width > 4 instead of >= 4 when writing upper 32 bytes.
- This improves the code but will not affect functionality.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Mark Wilson <Mark.Wilson@amd.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3584
Function AsmCpuid should first check the value for Basic CPUID Information.
The fix is to update the mPatchCetSupported judgment statement.
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Sheng W <w.sheng@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3508
Sort the CpuCacheInfo array by CPU package ID, core type, cache level
and cache type.
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
When enter SMM exception, there will be a stack switch only if the IST
field of the interrupt gate is set. When CET shadow stack feature is
enabled, if there is a stack switch between SMM exception and SMM, the
shadow stack token busy bit needs to be cleared when return from SMM
exception to SMM. In UEFI BIOS, only page fault exception does the stack
swith when SMM shack guard feature is enabled. The condition of clear
shadow stack token busy bit should be SMM stack guard enabled, CET shadows
stack feature enabled and page fault exception.
The shadow stack token should be initialized by UINT64.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3462
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Qihua Zhuang <qihua.zhuang@intel.com>
Cc: Daquan Dong <daquan.dong@intel.com>
Cc: Justin Tong <justin.tong@intel.com>
Cc: Tom Xu <tom.xu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3424
Processor location information check needs to updated
When Core 0 is disabled.
In C1e.c, change MSR_FEATURE_CONFIG to MSR_NEHALEM_POWER_CTL in comments
to match the correct MSR name.
Signed-off-by: Daoxiang Li <daoxiang.li@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3324
The SEV-ES stacks currently share a page with the reset code and data.
Separate the SEV-ES stacks from the reset vector code and data to avoid
possible stack overflows from overwriting the code and/or data.
When SEV-ES is enabled, invoke the GetWakeupBuffer() routine a second time
to allocate a new area, below the reset vector and data.
Both the PEI and DXE versions of GetWakeupBuffer() are changed so that
when PcdSevEsIsEnabled is true, they will track the previous reset buffer
allocation in order to ensure that the new buffer allocation is below the
previous allocation. When PcdSevEsIsEnabled is false, the original logic
is followed.
Fixes: 7b7508ad78
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Marvin Häuser <mhaeuser@posteo.de>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <3cae2ac836884b131725866264e0a0e1897052de.1621024125.git.thomas.lendacky@amd.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
5-level paging can be enabled on CPU which supports up to 52 physical
address size. But when the feature was enabled, the 48 address size
limit was not removed and the 5-level paging testing didn't access
address >= 2^48. So the issue wasn't detected until recently an
address >= 2^48 is accessed.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3373
An update to expand the mExceptionNameStr array failed to add a comma
after original last entry, therefore causing the #CP name to have extra
incorrect characters and the remaining entries to be indexed incorrectly
(off by 1).
Add a comma after the "#CP" entry to resolve this issue.
Fixes: 5277540e37
Cc: Allen Wynn <Allen_Wynn@Dell.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <40e55bf6563ed8ea4962a1219efbe1ab77a56cc4.1620919615.git.thomas.lendacky@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3385
A VS2012 build fails with a cast conversion warning when the SEV-ES work
area PCD is cast as a pointer to the SEV_ES_AP_JMP_FAR type.
When casting from a PCD value to a pointer, the cast should first be done
to a UINTN and then to the pointer. Update the code to perform a cast to
a UINTN before casting to a pointer to the SEV_ES_AP_JMP_FAR type.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Fixes: 7b7508ad78
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <c89bc819856d448360430c32cb3833a9667f987b.1620656694.git.thomas.lendacky@amd.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3300
Current implementation of SetStaticPageTable routine in PiSmmCpuDxeSmm
driver will check a global variable mPhysicalAddressBits, and eventually
cap any value larger than 39 at 39.
This global variable is used in ConvertMemoryPageAttributes, which backs
SmmSetMemoryAttributes and SmmClearMemoryAttributes. Thus for a processor
that supports more than 39 bits width, trying to mark page table regions
higher than 39-bit will always return EFI_UNSUPPROTED.
This change updated the interface of SetStaticPageTable function to take
PhysicalAddressBits as an input parameter, in order to avoid changing/
accessing the global variable.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Fixes: 4eee0cc7cc
Signed-off-by: Kun Qin <kuqin12@gmail.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3283
Current SMM Save State routine does not check the number of bytes to be
read, when it comse to read IO_INFO, before casting the incoming buffer
to EFI_SMM_SAVE_STATE_IO_INFO. This could potentially cause memory
corruption due to extra bytes are written out of buffer boundary.
This change adds a width check before copying IoInfo into output buffer.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210406195254.1018-2-kuqin12@gmail.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2832
Update LIBRARY_CLASS of BaseCpuTimerLib to remove the usage limitation,
otherwise the Base instance cannot be used in some types of modules.
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2832
1. Remove PEI instance(PeiCpuTimerLib).
PeiCpuTimerLib is currently designed to save time by getting CPU TSC
frequency from Hob. BaseCpuTimerLib is designed to calculate TSC frequency
by using CPUID[15h] each time.
The time it takes to find CpuCrystalFrequencyHob (about 2000ns) is much
longer than it takes to calculate TSC frequency with CPUID[15h] (about
450ns), which means using BaseCpuTimerLib to trigger a delay is more
accurate than using PeiCpuTimerLib, recommend to use BaseCpuTimerLib
instead of PeiCpuTimerLib.
2. Remove DXE instance(DxeCpuTimerLib).
DxeCpuTimerLib is designed to calculate TSC frequency with CPUID[15h] in
its constructor function, then save it in a global variable. For this
design, once the driver containing this instance is running, this
constructor function is called, it will take extra time to calculate TSC
frequency.
The time it takes to get TSC frequency from global variable is shorter
than it takes to calculate TSC frequency with CPUID[15h], but 450ns is a
short time, the impact on the platform is very limited.
In addition, in order to simplify the code, recommend to use
BaseCpuTimerLib instead of DxeCpuTimerLib.
I did some experiments on one server platform and collected following data:
1. Average time required to find CpuCrystalFrequencyHob: about 2000 ns.
2. Average time required to find the last Hob: about 2700 ns.
2. Average time required to calculate TSC frequency: about 450 ns.
Reference code:
//
// Calculate average time required to find Hob.
//
DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] GetPerformanceCounterFrequency - GetFirstGuidHob (1000 cycles)\n"));
Ticks1 = AsmReadTsc();
for (i = 0; i < 1000; i++) {
GuidHob = GetFirstGuidHob (&mCpuCrystalFrequencyHobGuid);
}
Ticks2 = AsmReadTsc();
if (GuidHob == NULL) {
DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] - CpuCrystalFrequencyHob can not be found!\n"));
} else {
DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] - Average time required to find Hob = %d ns\n", \
DivU64x32(DivU64x64Remainder(MultU64x32((Ticks2 - Ticks1), 1000000000), *CpuCrystalCounterFrequency, NULL), 1000)));
}
//
// Calculate average time required to calculate CPU frequency.
//
DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] GetPerformanceCounterFrequency - CpuidCoreClockCalculateTscFrequency (1000 cycles)\n"));
Ticks1 = AsmReadTsc();
for (i = 0; i < 1000; i++) {
Freq = CpuidCoreClockCalculateTscFrequency ();
}
Ticks2 = AsmReadTsc();
DEBUG((DEBUG_ERROR, "[PeiCpuTimerLib] - Average time required to calculate TSC frequency = %d ns\n", \
DivU64x32(DivU64x64Remainder(MultU64x32((Ticks2 - Ticks1), 1000000000), *CpuCrystalCounterFrequency, NULL), 1000)));
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3246
MdeLibs.dsc.inc was added for some basic/default library
instances provided by MdePkg and RegisterFilterLibNull Library
was also added into it as the first version of MdeLibs.dsc.inc.
So update platform dsc to consume MdeLibs.dsc.inc for
RegisterFilterLibNull which will be consumed by IoLib and BaseLib.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
The comments in PiSmmCommunicationPei.c describe the whole memory
layout of the SMRAM regarding the SMM communication.
But SHA-1: 8b1d149390
PiSmmCommunicationSmm: Deprecate SMM Communication ACPI Table
removed the code that produces the ACPI Table.
This change updates the accordingly comments.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3233
GDT needs to be allocated below 4GB in 64bit environment
because AP needs it for entering to protected mode.
CPU running in big real mode cannot access above 4GB GDT.
But CpuDxe driver contains below code:
gdt = AllocateRuntimePool (sizeof (GdtTemplate) + 8);
.....
gdtPtr.Base = (UINT32)(UINTN)(VOID*) gdt;
The AllocateRuntimePool() may allocate memory above 4GB.
Thus, we cannot use AllocateRuntimePool (), instead,
we should use AllocatePages() to make sure GDT is below 4GB space.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
The change doesn't impact any functionality.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
MpInitLib contains a function MicrocodeDetect() which is called by
all threads as an AP procedure.
Today this function contains below code:
if (CurrentRevision != LatestRevision) {
AcquireSpinLock(&CpuMpData->MpLock);
DEBUG ((
EFI_D_ERROR,
"Updated microcode signature [0x%08x] does not match \
loaded microcode signature [0x%08x]\n",
CurrentRevision, LatestRevision
));
ReleaseSpinLock(&CpuMpData->MpLock);
}
When the if-check is passed, the code may call into PEI services:
1. AcquireSpinLock
When the PcdSpinTimeout is not 0, TimerLib
GetPerformanceCounterProperties() is called. And some of the
TimerLib implementations would get the information cached in
HOB. But AP procedure cannot call PEI services to retrieve the
HOB list.
2. DEBUG
Certain DebugLib relies on ReportStatusCode services and the
ReportStatusCode PPI is retrieved through the PEI services.
DebugLibSerialPort should be used.
But when SerialPortLib is implemented to depend on PEI services,
even using DebugLibSerialPort can still cause AP calls PEI
services resulting hang.
It causes a lot of debugging effort on the platform side.
There are 2 options to fix the problem:
1. make sure platform DSC chooses the proper DebugLib and set the
PcdSpinTimeout to 0. So that AcquireSpinLock and DEBUG don't call
PEI services.
2. remove the AcquireSpinLock and DEBUG call from the procedure.
Option #2 is preferred because it's not practical to ask every
platform DSC to be written properly.
Following option #2, there are two sub-options:
2.A. Just remove the if-check.
2.B. Capture the CurrentRevision and ExpectedRevision in the memory
for each AP and print them together from BSP.
The patch follows option 2.B.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3265
Support collecting cache associative type in CpuCacheInfoLib.
This prevents the user from using additional code to obtain the
same information.
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3199
When Token points to mSmmStartupThisApToken, this routine is called
from SmmStartupThisAp() in non-blocking mode due to
PcdCpuSmmBlockStartupThisAp == FALSE.
In this case, caller wants to startup AP procedure in non-blocking
mode and cannot get the completion status from the Token because there
is no way to return the Token to caller from SmmStartupThisAp().
Caller needs to use its specific way to query the completion status.
There is no need to allocate a token for such case so the 3 overheads
can be avoided:
1. Call AllocateTokenBuffer() when there is no free token.
2. Get a free token from the token buffer.
3. Call ReleaseToken() in APHandler().
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3218
Adds an INF for StandaloneMmCpuFeaturesLib, which supports building
the SmmCpuFeaturesLib code for Standalone MM. Minimal code changes
are made to allow reuse of existing code for Standalone MM.
The original INF file names are left intact (continue to use SMM
terminology) to retain backward compatibility with platforms that
use those INFs. Similarly, the pre-existing C file names are
unchanged to be consistent with the INF file names.
Note that all references in library source files to PiSmm.h have
been changed to PiMm.h for consistency.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Message-Id: <20210217213227.1277-6-mikuback@linux.microsoft.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=3218
Adds a new function called GetCpuMaxLogicalProcessorNumber() to
return the number of maximum CPU logical processors (currently
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber).
This allows the the mechanism used to retrieve the CPU maximum
logical processor number to be abstracted from the logic that
needs the value.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210217213227.1277-5-mikuback@linux.microsoft.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
There's currently two library instances:
1. SmmCpuFeaturesLib
2. SmmCpuFeaturesLibStm
There's two constructor functions:
1. SmmCpuFeaturesLibConstructor()
2. SmmCpuFeaturesLibStmConstructor()
SmmCpuFeaturesLibConstructor() is called by
SmmCpuFeaturesLibStmConstructor() since the functionality in that
function is required by both library instances.
The declaration for SmmCpuFeaturesLibConstructor() is embedded in
"SmmStm.c" instead of being declared in a header file. Further,
that constructor function is called by the STM specific constructor.
This change moves the common code to a function called
CpuFeaturesLibInitialization() which is declared in an internal
library header file "CpuFeaturesLib.h". Each constructor simply
calls this function to perform the common functionality.
Additionally, SmmCpuFeaturesLibConstructor() is moved from
SmmCpuFeaturesLibNoStm.c into a instance-specific file allowing
SmmCpuFeaturesLibNoStm.c to contain no STM implementation agnostic
to a particular library instance.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210217213227.1277-4-mikuback@linux.microsoft.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
This change renames SmmCpuFeaturesLib.c to SmmCpuFeaturesLibCommon.c
to better convey that this file contains library implementation
common to all library instances.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Message-Id: <20210217213227.1277-3-mikuback@linux.microsoft.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
FinishSmmCpuFeaturesInitializeProcessor() is a multi-instance
internal library function that is currently not declared in a
header file but embedded in "SmmCpuFeaturesLib.c".
This change cleans up the declaration moving it to a new header
file "CpuFeaturesLib.h" and removing the local declaration in
"SmmCpuFeaturesLib.c".
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210217213227.1277-2-mikuback@linux.microsoft.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
[lersek@redhat.com: replace the guard macro "_CPU_FEATURES_LIB_H_" with
"CPU_FEATURES_LIB_H_", for fixing ECC 8003, per commit 6ffbb3581a]
The Lock is no longer needed since "LOCK XADD" was used in
MpFuncs.nasm for ApIndex atomic increment.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
In Windows environment, "dumpbin /disasm" is used to verify the
disassembly before and after using NASM struc doesn't change.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
In function InitGdt(), SmiPFHandler() and Gen4GPageTable(), it uses
CpuIndex * mSmmStackSize to get the SMM stack address offset for
multi processor. It misses the SMM Shadow Stack Size. Each processor
will use mSmmStackSize + mSmmShadowStackSize in the memory.
It should use CpuIndex * (mSmmStackSize + mSmmShadowStackSize) to get
this SMM stack address offset. If mSmmShadowStackSize > 0 and multi
processor enabled, it will get the wrong offset value.
CET shadow stack feature will set the value of mSmmShadowStackSize.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3237
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Roger Feng <roger.feng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
If CET shadows stack feature enabled in SMM and stack switch is enabled.
When code execute from SMM handler to SMM exception, CPU will check SMM
exception shadow stack token busy bit if it is cleared or not.
If it is set, it will trigger #DF exception.
If it is not set, CPU will set the busy bit when enter SMM exception.
So, the busy bit should be cleared when return back form SMM exception to
SMM handler. Otherwise, keeping busy bit 1 will cause to trigger #DF
exception when enter SMM exception next time.
So, we use instruction SAVEPREVSSP, CLRSSBSY and RSTORSSP to clear the
shadow stack token busy bit before RETF instruction in SMM exception.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3192
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Roger Feng <roger.feng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
When AP firstly wakes up, MpFuncs.nasm contains below logic to assign
an unique ApIndex to each AP according to who comes first:
---ASM---
TestLock:
xchg [edi], eax
cmp eax, NotVacantFlag
jz TestLock
mov ecx, esi
add ecx, ApIndexLocation
inc dword [ecx]
mov ebx, [ecx]
Releaselock:
mov eax, VacantFlag
xchg [edi], eax
---ASM END---
"lock inc" cannot be used to increase ApIndex because not only the
global ApIndex should be increased, but also the result should be
stored to a local general purpose register EBX.
This patch learns from the NASM implementation of
InternalSyncIncrement() to use "XADD" instruction which can increase
the global ApIndex and store the original ApIndex to EBX in one
instruction.
With this patch, OVMF when running in a 255 threads QEMU spends about
one second to wakeup all APs. Original implementation needs more than
10 seconds.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@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=1614
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3160
The GDT still in flash with commit 60b12e69fb
after TempRamDone
So move the action to TempRamDone event to avoid reading GDT from flash.
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3195
Support system has no enabled AP case in DxeCpuCacheInfoLib.
Otherwise, if the system only has 1 BSP without any enabled AP,
UEFI POST hangs when invoking StartupAllAPs protocol because
EFI_NOT_STARTED is returned.
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3190
Add MpService dependency to enforce the executability of CpuCacheInfoLib.
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
This change of SmmCpuExceptionHandlerLib adds support for StandaloneMm
components to allow x64 StandaloneMm environment setting up exception
handlers.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Kun Qin <kun.q@outlook.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
This change adds a new CpuIo driver instance for MM_STANDALONE type. The
new driver entrypoint is implemented in a separate file to match the
interface definition of MM_STANDALONE modules.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Kun Qin <kun.q@outlook.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
This change abstracts CpuIo2Smm driver entrypoint into separate file and
moves functions/definitions that are not substantially specific to
Traditional MM (SMM) into CpuIo2Mm.* in order to set ways for Standalone
MM support in the future.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Kun Qin <kun.q@outlook.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
This change replaced gSmst with gMmst to support broader compatibility
under MM environment for CpuIo2Smm driver.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Kun Qin <kun.q@outlook.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3179
When BSP first time wakes all APs, each AP atomically increases
CpuMpData->CpuCount and CpuMpData->FinishedCount.
Each AP atomically increases CpuMpData->NumApsExecuting
in early assembly code and decreases it before it enters to HLT or
MWAIT state.
Putting them together, the 3 variables are changed in the following order:
1. NumApsExecuting++ // in assembly
2. CpuCpunt++
4. FinishedCount++
3. NumApsExecuting-- // in C
BSP waits for a certain timeout and then polls NumApsExecuting
until it drops to zero. It assumes all APs are waken up concurrently
and NumApsExecuting only drops to zero when all APs have checked in.
Then it additionally waits for FinishedCount == CpuCount - 1. (FinishedCount doesn't include BSP while CpuCount includes BSP.)
There is no need to additionally wait for
FinishedCount == CpuCount - 1 because when NumApsExecuting == 0,
the number of increament of FinishedCount and CpuCount should equal.
This patch simplifies the code to remove "CpuCount++" in
ApWakeupFunction() and
assigns FinishedCount + 1 to CpuCount after WakeUpAP().
Signed-off-by: Ray Ni <ray.ni@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=3105
Change and add some macro definitions about
CPUID_HYBRID_INFORMATION Leaf(1Ah).
Signed-off-by: Jason Lou <yun.lou@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3182
Fix the order of operations in ApWakeupFunction() when PcdCpuApLoopMode
is set to HLT mode that uses INIT-SIPI-SIPI to wake APs. In this mode,
volatile state is restored and saved each time a INIT-SIPI-SIPI is sent
to an AP to request a function to be executed on the AP. When the
function is completed the volatile state of the AP is saved. However,
the counters NumApsExecuting and FinishedCount are updated before
the volatile state is saved. This allows for a race condition window
for the BSP that is waiting on these counters to request a new
INIT-SIPI-SIPI before all the APs have completely saved their volatile
state. The fix is to save the AP volatile state before updating the
NumApsExecuting and FinishedCount counters.
Cc: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3167
When StackGuard is enabled, the CpuMp driver allocates
known good stacks for all CPUs for DF# and PF# exceptions.
It uses AllocatePool to do so.
The size needed equals to 64KB
= StackSize (2K) * ExceptionNumber (2) * NumberOfProcessors (16)
However, AllocatePool max allocation size is less than 64K.
To fix the issue, AllocatePages() is used.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
NumberOfCpus local variable in GetAcpiCpuData will be uninitialized
when CpuS3DataDxe runs before DxeRegisterCpuFeaturesLib (linked by
CpuFeaturesDxe) because there is no code to initialize it at
(AcpiCpuData != NULL) execution path.
The issue is exposed after cefad282fb
and 38ee7bafa7.
There was negligence in that code review.
One further topic may be "Could EDK2 CI be enhanced to catch this kind
of uninitialized local variable case?". :)
This patch fixes this regression issue.
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>
Message-Id: <20210121093944.1621-1-star.zeng@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
CpuS3DataDxe allocates the "RegisterTable" and "PreSmmInitRegisterTable"
arrays in ACPI_CPU_DATA just so every processor in the system can have its
own empty register table, matched by APIC ID. This has never been useful
in practice.
Given commit e992cc3f48 ("UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM
consumption in CpuS3.c", 2021-01-11), simply leave both
"AcpiCpuData->RegisterTable" and "AcpiCpuData->PreSmmInitRegisterTable"
initialized to the zero address. This simplifies the driver, and saves
both normal RAM (boot services data type memory) and -- in PiSmmCpuDxeSmm
-- SMRAM.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Message-Id: <20210119155440.2262-4-lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
After commit e992cc3f48 ("UefiCpuPkg PiSmmCpuDxeSmm: Reduce SMRAM
consumption in CpuS3.c", 2021-01-11), it is valid for a CPU S3 Data DXE
Driver to set "ACPI_CPU_DATA.PreSmmInitRegisterTable" and/or
"ACPI_CPU_DATA.RegisterTable" to 0, in case none of the CPUs needs a
register table of the corresponding kind, during S3 resume.
Document this fact in the "UefiCpuPkg/Include/AcpiCpuData.h" header file.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210119155440.2262-3-lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
There are lots of fields in ACPI_CPU_DATA structure while only
followings are accessed by CpuFeature infra:
* NumberOfCpus
* PreSmmInitRegisterTable // pointer
* RegisterTable // pointer
* CpuStatus
* ApLocation // pointer
So it's possible that an implementation of CpuS3DataDxe doesn't
allocate memory for PreSmmInitRegisterTable/RegisterTable/ApLocation.
This patch handles the case when CpuS3DataDxe doesn't allocate
memory for PreSmmInitRegisterTable/RegisterTable.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3159
Signed-off-by: Ray Ni <ray.ni@intel.com>
[lersek@redhat.com: update CC list, add BZ reference, add my S-o-b]
[lersek@redhat.com: deal with RegisterTable and PreSmmInitRegisterTable
being zero independently of each other; replacing the ASSERT()]
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210119155440.2262-2-lersek@redhat.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3105
This new library uses a platform agnostic algorithm to get CPU
cache information. It provides user with an API(GetCpuCacheInfo)
to get detailed CPU cache information by each package, each core
type included in this package, and each cache level & type.
This library can be used by code that produces SMBIOS_TABLE_TYPE7
SMBIOS table.
Signed-off-by: Jason Lou <yun.lou@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3084
When DXE drivers are dispatched above 4GB memory in 64bit
mode, the address setCodeSelectorLongJump in stack will
be override by parameter. Jump to Qword is not supported
by some processors. So use "o64 retf" instead.
Signed-off-by: Guo Dong <guo.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Tom Lendacky <thomas.lendacky@amd.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Tested-by: James Bottomley <jejb@linux.ibm.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
This patch fixed the hang in UEFICpuPkg when it is dispatched above 4GB.
In UEFI BIOS case CpuInfoInHob is provided to DXE under 4GB from PEI.
When using UEFI payload and bootloaders, CpuInfoInHob will be allocated
above 4GB since it is not provided from bootloader. so we need update
the code to make sure this hob could be accessed correctly in this case.
Signed-off-by: Guo Dong <guo.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
This patch makes two refinements to reduce SMRAM consumption in CpuS3.c.
1. Only do CopyRegisterTable() when register table is not empty,
IsRegisterTableEmpty() is created to check whether the register table
is empty or not.
Take empty PreSmmInitRegisterTable as example, about 24K SMRAM consumption
could be reduced when mAcpiCpuData.NumberOfCpus=1024.
sizeof (CPU_REGISTER_TABLE) = 24
mAcpiCpuData.NumberOfCpus = 1024 = 1K
mAcpiCpuData.NumberOfCpus * sizeof (CPU_REGISTER_TABLE) = 24K
2. Only copy table entries buffer instead of whole buffer.
AllocatedSize in SourceRegisterTableList is the whole buffer size.
Actually, only the table entries buffer needs to be copied, and the size
is TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY).
Take AllocatedSize=0x1000=4096, TableLength=100 and NumberOfCpus=1024 as example,
about 1696K SMRAM consumption could be reduced.
sizeof (CPU_REGISTER_TABLE_ENTRY) = 24
TableLength = 100
TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY) = 2400
AllocatedSize = 0x1000 = 4096
AllocatedSize - TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY) = 4096 - 2400 = 1696
NumberOfCpus = 1024 = 1K
NumberOfCpus * (AllocatedSize - TableLength * sizeof (CPU_REGISTER_TABLE_ENTRY)) = 1696K
This patch also corrects the CopyRegisterTable() function description.
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210111015419.28368-1-star.zeng@intel.com>
CpuInfo.First stores whether the current thread belongs to the first
package in the platform, first core in a package, first thread in a
core.
But the time complexity of original algorithm to calculate the
CpuInfo.First is O (n) * O (p) * O (c).
n: number of processors
p: number of packages
c: number of cores per package
The patch trades time with space by storing the first package, first
core per package, first thread per core in an array.
The time complexity becomes O (n).
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Cc: Yun Lou <yun.lou@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
The required buffer size for InitOrder will be 96K when NumberOfCpus=1024.
sizeof (CPU_FEATURES_INIT_ORDER) = 96
NumberOfCpus = 1024 = 1K
sizeof (CPU_FEATURES_INIT_ORDER) * NumberOfCpus = 96K
AllocateZeroPool() will call to PeiServicesAllocatePool() which will use
EFI_HOB_MEMORY_POOL to management memory pool.
EFI_HOB_MEMORY_POOL.Header.HobLength is UINT16 type, so there is no way
for AllocateZeroPool() to allocate > 64K memory.
So AllocateZeroPool() could not be used anymore for the case above or
even bigger required buffer size.
This patch updates the code to use AllocatePages() instead of
AllocateZeroPool() to allocate buffer for InitOrder.
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3084
When DXE drivers are dispatched above 4GB memory and
the system is already in 64bit mode, the address
setCodeSelectorLongJump in stack will be override
by parameter. so change to use 64bit address and
jump to qword address.
Signed-off-by: Guo Dong <guo.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Today's code assumes every core contains the same number of threads.
It's not always TRUE for certain model.
Such assumption causes system hang when thread count per core
is different and there is core or package dependency between CPU
features (using CPU_FEATURE_CORE_BEFORE/AFTER,
CPU_FEATURE_PACKAGE_BEFORE/AFTER).
The change removes such assumption by calculating the actual thread
count per package and per core.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Yun Lou <yun.lou@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
When trying to get page table base, if mInternalCr3 is zero, it will use
the page table from CR3, and reflect the page table depth by CR4 LA57 bit.
If mInternalCr3 is non zero, it will use the page table from mInternalCr3
and reflect the page table depth of mInternalCr3 at same time.
In the case of X64, we use m5LevelPagingNeeded to reflect the depth of
the page table. And in the case of IA32, it will not the page table depth
information.
This patch is a bug fix when enable CET feature with 5 level paging.
The SMM page tables are allocated / initialized in PiCpuSmmEntry().
When CET is enabled, PiCpuSmmEntry() must further modify the attribute of
shadow stack pages. This page table is not set to CR3 in PiCpuSmmEntry().
So the page table base address is set to mInternalCr3 for modifty the
page table attribute. It could not use CR4 LA57 bit to reflect the
page table depth for mInternalCr3.
So we create a architecture-specific implementation GetPageTable() with
2 output parameters. One parameter is used to output the page table
address. Another parameter is used to reflect if it is 5 level paging
or not.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3015
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Change the variable name from mInternalGr3 to mInternalCr3.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3015
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jiewen Yao <jiewen.yao@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>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008
Set the SEV-ES reset stack address for an AP based on the processor number
instead of the APIC ID in case the APIC IDs are not zero-based and densely
packed/enumerated. This will ensure an AP reset stack address does not get
set outside of the AP reset stack memory allocation.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <24866de07d2a954dec71df70972f1851273020d8.1604685192.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008
The QemuFlashPtrWrite() flash services runtime uses the GHCB and VmgExit()
directly to perform the flash write when running as an SEV-ES guest. If an
interrupt arrives between VmgInit() and VmgExit(), the Dr7 read in the
interrupt handler will generate a #VC, which can overwrite information in
the GHCB that QemuFlashPtrWrite() has set. This has been seen with the
timer interrupt firing and the CpuExceptionHandlerLib library code,
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/
Xcode5ExceptionHandlerAsm.nasm and
ExceptionHandlerAsm.nasm
reading the Dr7 register while QemuFlashPtrWrite() is using the GHCB. In
general, it is necessary to protect the GHCB whenever it is used, not just
in QemuFlashPtrWrite().
Disable interrupts around the usage of the GHCB by modifying the VmgInit()
and VmgDone() interfaces:
- VmgInit() will take an extra parameter that is a pointer to a BOOLEAN
that will hold the interrupt state at the time of invocation. VmgInit()
will get and save this interrupt state before updating the GHCB.
- VmgDone() will take an extra parameter that is used to indicate whether
interrupts are to be (re)enabled. Before exiting, VmgDone() will enable
interrupts if that is requested.
Fixes: 437eb3f7a8
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Acked-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <c326a4fd78253f784b42eb317589176cf7d8592a.1604685192.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008
All fields that are set in the GHCB should have their associated bit in
the GHCB ValidBitmap field set. Add support to set the bits for the
software exit information fields when performing a VMGEXIT (SwExitCode,
SwExitInfo1, SwExitInfo2).
Fixes: 20da7ca42a
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Acked-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <6e11dd7e161bddeacc3fb4817467cef24510c31c.1604685192.git.thomas.lendacky@amd.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008
In upcoming patches, the setting of the bits in the GHCB ValidBitmap will
be performed in multiple places. In order to reduce code duplication, add
an interface, VmgSetOffsetValid(), to VmgExitLib library to perform this
function. Also, to keep management of the ValidBitmap within the library,
add an inteface, VmgIsOffsetValid(), to return whether the bit in the
ValidBitmap is set for a specified offset.
The new VmgSetOffsetValid() function is a VOID function and will be an
empty function in the VmgExitLibNull implementation of the VmgExitLib
library.
The new VmgIsOffsetValid() function returns a BOOLEAN to indicate if the
offset is valid. This will always return FALSE in the VmgExitLibNull
implementation of the VmgExitLib library.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Acked-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <0bcb2373f8c6e0171ae277d3d7c2eb284621355e.1604685192.git.thomas.lendacky@amd.com>
The AP reset vector stack allocation is only required if running as an
SEV-ES guest. Since the reset vector allocation is below 1MB in memory,
eliminate the requirement for bare-metal systems and non SEV-ES guests
to allocate the extra stack area, which can be large if the
PcdCpuMaxLogicalProcessorNumber value is large, and also remove the
CPU_STACK_ALIGNMENT alignment.
Fixes: 7b7508ad78 ("UefiCpuPkg: Allow AP booting under SEV-ES")
Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <21345cdbc906519558202b3851257ca07b9239ba.1600884239.git.thomas.lendacky@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
[lersek@redhat.com: supply missing space character after "PcdGet32"]
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2883
MpServices Ppi can be replaced by MpServices2 Ppi and MpServices2
Ppi is mandatory for RegisterCpuFeaturesLib functionality,
basing on this we can drop MpServices Ppi usage from the library
and the constraint that both Ppis must be installed.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2901
The DoDecrement variable in ApWakeupFunction () wasn't always being
initialized. Update the code to always fully initialize it.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <76a9f18992475b915e5f8457704676067210cacf.1597935198.git.thomas.lendacky@amd.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Tested-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Add configuration ExceptionList and IgnoreFiles for package config
files. So users can rely on this to ignore some Ecc issues.
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>
Acked-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
Before UEFI transfers control to the OS, it must park the AP. This is
done using the AsmRelocateApLoop function to transition into 32-bit
non-paging mode. For an SEV-ES guest, a few additional things must be
done:
- AsmRelocateApLoop must be updated to support SEV-ES. This means
performing a VMGEXIT AP Reset Hold instead of an MWAIT or HLT loop.
- Since the AP must transition to real mode, a small routine is copied
to the WakeupBuffer area. Since the WakeupBuffer will be used by
the AP during OS booting, it must be placed in reserved memory.
Additionally, the AP stack must be located where it can be accessed
in real mode.
- Once the AP is in real mode it will transfer control to the
destination specified by the OS in the SEV-ES AP Jump Table. The
SEV-ES AP Jump Table address is saved by the hypervisor for the OS
using the GHCB VMGEXIT AP Jump Table exit code.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
Typically, an AP is booted using the INIT-SIPI-SIPI sequence. This
sequence is intercepted by the hypervisor, which sets the AP's registers
to the values requested by the sequence. At that point, the hypervisor can
start the AP, which will then begin execution at the appropriate location.
Under SEV-ES, AP booting presents some challenges since the hypervisor is
not allowed to alter the AP's register state. In this situation, we have
to distinguish between the AP's first boot and AP's subsequent boots.
First boot:
Once the AP's register state has been defined (which is before the guest
is first booted) it cannot be altered. Should the hypervisor attempt to
alter the register state, the change would be detected by the hardware
and the VMRUN instruction would fail. Given this, the first boot for the
AP is required to begin execution with this initial register state, which
is typically the reset vector. This prevents the BSP from directing the
AP startup location through the INIT-SIPI-SIPI sequence.
To work around this, the firmware will provide a build time reserved area
that can be used as the initial IP value. The hypervisor can extract this
location value by checking for the SEV-ES reset block GUID that must be
located 48-bytes from the end of the firmware. The format of the SEV-ES
reset block area is:
0x00 - 0x01 - SEV-ES Reset IP
0x02 - 0x03 - SEV-ES Reset CS Segment Base[31:16]
0x04 - 0x05 - Size of the SEV-ES reset block
0x06 - 0x15 - SEV-ES Reset Block GUID
(00f771de-1a7e-4fcb-890e-68c77e2fb44e)
The total size is 22 bytes. Any expansion to this block must be done
by adding new values before existing values.
The hypervisor will use the IP and CS values obtained from the SEV-ES
reset block to set as the AP's initial values. The CS Segment Base
represents the upper 16 bits of the CS segment base and must be left
shifted by 16 bits to form the complete CS segment base value.
Before booting the AP for the first time, the BSP must initialize the
SEV-ES reset area. This consists of programming a FAR JMP instruction
to the contents of a memory location that is also located in the SEV-ES
reset area. The BSP must program the IP and CS values for the FAR JMP
based on values drived from the INIT-SIPI-SIPI sequence.
Subsequent boots:
Again, the hypervisor cannot alter the AP register state, so a method is
required to take the AP out of halt state and redirect it to the desired
IP location. If it is determined that the AP is running in an SEV-ES
guest, then instead of calling CpuSleep(), a VMGEXIT is issued with the
AP Reset Hold exit code (0x80000004). The hypervisor will put the AP in
a halt state, waiting for an INIT-SIPI-SIPI sequence. Once the sequence
is recognized, the hypervisor will resume the AP. At this point the AP
must transition from the current 64-bit long mode down to 16-bit real
mode and begin executing at the derived location from the INIT-SIPI-SIPI
sequence.
Another change is around the area of obtaining the (x2)APIC ID during AP
startup. During AP startup, the AP can't take a #VC exception before the
AP has established a stack. However, the AP stack is set by using the
(x2)APIC ID, which is obtained through CPUID instructions. A CPUID
instruction will cause a #VC, so a different method must be used. The
GHCB protocol supports a method to obtain CPUID information from the
hypervisor through the GHCB MSR. This method does not require a stack,
so it is used to obtain the necessary CPUID information to determine the
(x2)APIC ID.
The new 16-bit protected mode GDT entry is used in order to transition
from 64-bit long mode down to 16-bit real mode.
A new assembler routine is created that takes the AP from 64-bit long mode
to 16-bit real mode. This is located under 1MB in memory and transitions
from 64-bit long mode to 32-bit compatibility mode to 16-bit protected
mode and finally 16-bit real mode.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
When starting APs in an SMP configuration, the AP needs to know if it is
running as an SEV-ES guest in order to assign a GHCB page.
Add a field to the CPU_MP_DATA structure that will indicate if SEV-ES is
enabled. This new field is set during MP library initialization with the
PCD value PcdSevEsIsEnabled. This flag can then be used to determine if
SEV-ES is enabled.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
A hypervisor is not allowed to update an SEV-ES guests register state,
so when booting an SEV-ES guest AP, the hypervisor is not allowed to
set the RIP to the guest requested value. Instead, an SEV-ES AP must be
transition from 64-bit long mode to 16-bit real mode in response to an
INIT-SIPI-SIPI sequence. This requires a 16-bit code segment descriptor.
For PEI, create this descriptor in the reset vector GDT table. For DXE,
create this descriptor from the newly reserved entry at location 0x28.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
Create an SEV-ES workarea PCD. This PCD will be used for BSP communication
during SEC and for AP startup during PEI and DXE phases, the latter is the
reason for creating it in the UefiCpuPkg.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
Add base support to handle #VC exceptions. Update the common exception
handlers to invoke the VmgExitHandleVc () function of the VmgExitLib
library when a #VC is encountered. A non-zero return code will propagate
to the targeted exception handler.
Under SEV-ES, a DR7 read or write intercept generates a #VC exception.
To avoid exception recursion, a #VC exception will not try to read and
push the actual debug registers into the EFI_SYSTEM_CONTEXT_X64 struct
and instead push zeroes. The #VC exception handler does not make use of
the debug registers from the saved context and the exception processing
exit code does not attempt to restore the debug register values.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
To support handling #VC exceptions and issuing VMGEXIT instructions,
create a library with functions that can be used to perform these
#VC/VMGEXIT related operations. This includes functions for:
- Handling #VC exceptions
- Preparing for and issuing a VMGEXIT
- Performing MMIO-related write operations to support flash emulation
- Performing AP related boot opeations
The base functions in this driver will not do anything and will return
an error if a return value is required. It is expected that other packages
(like OvmfPkg) will create a version of the library to fully support an
SEV-ES guest.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
A new dynamic UefiCpuPkg PCD is needed to support SEV-ES under OVMF:
- PcdSevEsIsEnabled: BOOLEAN value used to indicate if SEV-ES is enabled
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
The unit test app supports running in 3 mode:
1. MtrrLibUnitTest generate-random-numbers
<path to MtrrLib/UnitTest/RandomNumber.c> <random-number count>
It generates random numbers and writes to RandomNumber.c.
2. MtrrLibUnitTest [<iterations>]
It tests MtrrLib APIs using configurations generated from static
numbers generated by mode #1.
This is the default execution mode running in CI environment.
3. MtrrLibUnitTest <iterations> random
It tests MtrrLib APIs using configurations generated from random
numbers.
This is what developers can use to test MtrrLib for regressions.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ming Shao <ming.shao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Add host based unit tests for the MtrrLib services.
The BaseLib services AsmCpuid(), AsmReadMsr64(), and
AsmWriteMsr64() are hooked and provide simple emulation
of the CPUID leafs and MSRs required by the MtrrLib to
run as a host based unit test.
Test cases are developed for each of the API.
For the most important APIs MtrrSetMemoryAttributesInMtrrSettings()
and MtrrSetMemoryAttributeInMtrrSettings(), random inputs are
generated and fed to the APIs to make sure the implementation is
good. The test application accepts an optional parameter which
specifies how many iterations of feeding random inputs to the two
APIs. The overall number of test cases increases when the iteration
increases. Default iteration is 10 when no parameter is specified.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Ming Shao <ming.shao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ming Shao <ming.shao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Most busy waits (spinlocks) in "UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c"
already call CpuPause() in their loop bodies; see SmmWaitForApArrival(),
APHandler(), and SmiRendezvous(). However, the "main wait" within
APHandler():
> //
> // Wait for something to happen
> //
> WaitForSemaphore (mSmmMpSyncData->CpuData[CpuIndex].Run);
doesn't do so, as WaitForSemaphore() keeps trying to acquire the semaphore
without pausing.
The performance impact is especially notable in QEMU/KVM + OVMF
virtualization with CPU overcommit (that is, when the guest has
significantly more VCPUs than the host has physical CPUs). The guest BSP
is working heavily in:
BSPHandler() [MpService.c]
PerformRemainingTasks() [PiSmmCpuDxeSmm.c]
SetUefiMemMapAttributes() [SmmCpuMemoryManagement.c]
while the many guest APs are spinning in the "Wait for something to
happen" semaphore acquisition, in APHandler(). The guest APs are
generating useless memory traffic and saturating host CPUs, hindering the
guest BSP's progress in SetUefiMemMapAttributes().
Rework the loop in WaitForSemaphore(): call CpuPause() in every iteration
after the first check fails. Due to Pause Loop Exiting (known as Pause
Filter on AMD), the host scheduler can favor the guest BSP over the guest
APs.
Running a 16 GB RAM + 512 VCPU guest on a 448 PCPU host, this patch
reduces OVMF boot time (counted until reaching grub) from 20-30 minutes to
less than 4 minutes.
The patch should benefit physical machines as well -- according to the
Intel SDM, PAUSE "Improves the performance of spin-wait loops". Adding
PAUSE to the generic WaitForSemaphore() function is considered a general
improvement.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1861718
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200729185217.10084-1-lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Add configuration IgnoreFiles for package config files.
So users can rely on this to skip license conflict for
some generated files.
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: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
To avoid the TOCTOU, enable paging and set Not Present flag so when
access any code in the flash range, it will trigger #PF exception.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
Adds a PEIM that republishes structures produced in SEC. This
is done because SEC modules may not be shadowed in some platforms
due to space constraints or special alignment requirements. The
SecMigrationPei module locates interfaces that may be published in
SEC and reinstalls the interface with permanent memory addresses.
This is important if pre-memory address access is forbidden after
memory initialization and data such as a PPI descriptor, PPI GUID,
or PPI inteface reside in pre-memory.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
Moves the GDT to permanent memory in a memory discovered
callback. This is done to ensure the GDT authenticated in
pre-memory is not fetched from outside a verified location
after the permanent memory transition.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2849
MtrrSetFixedMtrr() sets all the fixed MTRR settings.
But in fact MtrrSetAllMtrrs() is always used by callers to set all
MTRR settings including the fixed and variable ones.
The patch removes the unnecessary API MtrrSetFixedMtrr()
to simplify the MtrrLib API.
There is no code in edk2 and edk2-platforms repo that calls
MtrrSetFixedMtrr().
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2849
MtrrSetVariableMtrr() sets all the variable MTRR settings.
But in fact MtrrSetAllMtrrs() is always used by callers to set all
MTRR settings including the fixed and variable ones.
The patch removes the unnecessary API MtrrSetVariableMtrr() to
simplify the MtrrLib API.
There is no code in edk2 and edk2-platforms repo that calls
MtrrSetVariableMtrr().
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2849
MtrrGetVariableMtrr() returns all the variable MTRR settings.
But in fact MtrrGetAllMtrrs() and
MtrrGetMemoryAttributeInVariableMtrr() are used by callers to get the
MTRR settings. The former one returns both the fixed and variable
MTRR settings.
The patch removes the unnecessary API MtrrGetVariableMtrr() to
simplify the MtrrLib API.
There is no code in edk2 and edk2-platforms repo that calls
MtrrGetVariableMtrr().
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
AMD does not support MSR_IA32_MISC_ENABLE. Accessing that register
causes and exception on AMD processors. If Execution Disable is
supported, but if the processor is an AMD processor, skip manipulating
MSR_IA32_MISC_ENABLE[34] XD Disable bit.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
Message-Id: <20200622131825.1352-5-Garrett.Kirkendall@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Refactor StandardSignatureIsAuthenticAMD into BaseUefiCpuLib from
separate copies in BaseXApicLib, BaseXApicX2ApicLib, and MpInitLib.
This allows for future use of StandarSignatureIsAuthinticAMD without
creating more instances in other modules.
This function allows IA32/X64 code to determine if it is running on an
AMD brand processor.
UefiCpuLib is already included directly or indirectly in all modified
modules. Complete move is made in this change.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Message-Id: <20200622131825.1352-4-Garrett.Kirkendall@amd.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2776
Add a vector at 0xFF000 (0xFFFFF000) that can be used by Init-SIPI-SIPI
to start an AP before memory is initialized. This vector jumps into the
same SEC entry point as the ordinary reset vector, with a special value
of "AP" in the DI register. The platform-specific SEC code is expected
to check for that value and take a different path for APs, if this
feature is supported by the platform.
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Deric Cole <deric.cole@intel.com>
Intel SDM introduces 6-levels for describing the CPU topology:
* Package
* Module
* Tile
* Die
* Core
* Thread
A PI spec ECR was submitted to enhance CPU_MP PPI/Protocol to
support returning such information through GetProcessorInfo().
An accordingly change was implemented and pushed to edk2-staging.
Now the PI spec has been published.
The patch is cherry-picked from edk2-staging to edk2.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340
Now that an XCODE5 specific CpuExceptionHandlerLib library is in place,
revert the changes made to the ExceptionHandlerAsm.nasm in commit
2db0ccc2d7 ("UefiCpuPkg: Update CpuExceptionHandlerLib pass XCODE5 tool
chain") so that binary patching of flash code is not performed.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Acked-by: Bret Barkelew <bret.barkelew@microsoft.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <810f67d8604c054c09d17a22f0bcfaeb41ee8e3b.1588856809.git.thomas.lendacky@amd.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2340
Commit 2db0ccc2d7 ("UefiCpuPkg: Update CpuExceptionHandlerLib pass
XCODE5 tool chain") introduced binary patching into the exception handling
support. CPU exception handling is allowed during SEC and this results in
binary patching of flash, which should not be done.
Separate the changes from commit 2db0ccc2d7 into an XCODE5 toolchain
specific file, Xcode5ExceptionHandlerAsm.nasm, and create a new SEC INF
file for the XCODE5 version of CpuExceptionHandlerLib.
Since binary patching is allowed when running outside of flash, switch
the Dxe, Pei and Smm versions of the CpuExceptionHandlerLib over to use
the Xcode5ExceptionHandlerAsm.nasm file to retain current functionality.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Message-Id: <9075570487616c731033a5738f6a444a15d71b74.1588856809.git.thomas.lendacky@amd.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2678
This patch fixes a file permission issue introduced by accident.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Eric Dong <eric.dong@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2683
In PEI phase, AP already been waked up through ApInitConfig,
so it can directly wake up it through change wakup buffer
instead of use ApInitReconfig flag. It can save some time.
Change code to only use ApInitReconfig flag in DXE phase
which must need to update the wake up buffer.
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2683
This patch fixes an assertion because AP can't find the CpuMpData.
When AP is waken up through Init-Sipi-Sipi, AP's IDT should
be restored to pre-allocated buffer so AP can get the CpuMpData
through the IDT base address.
Current code already has logic to handle this when CpuMpData->
InitFlag is ApInitConfig but misses the logic
when CpuMpData->InitFlag is ApInitReconfig.
This patch fixes this gap.
Reviewed-by: Ray Ni <ray.ni@intel.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2632
Both PEI and DXE instances of the MpInitLib are using PcdLib APIs, but
none of them list the dependency of the PcdLib in INF & header files.
This commit will explicitly add such dependency in .H and .INF files.
Test done:
Library level build pass for VS2015x86 tool chain
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388
After remove Used parameter, below code in ResetTokens can also be
removed:
1. The RunningApCount parameter will be reset in GetFreeToken.
2. The ReleaseSpinLock should be called in ReleaseToken function,
Code in this function seems like a later fix if ReleaseToken not
Release it. We should remove code here and fix the real issue if
existed.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2388
After patch "UefiCpuPkg/PiSmmCpuDxeSmm: Improve the
performance of GetFreeToken()" which adds new parameter
FirstFreeToken, it's not need to use Uses parameter.
This patch used to remove this parameter.
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Today's GetFreeToken() runs at the algorithm complexity of O(n)
where n is the size of the token list.
The change introduces a new global variable FirstFreeToken and it
always points to the first free token. So the algorithm complexity
of GetFreeToken() decreases from O(n) to O(1).
The improvement matters when some SMI code uses StartupThisAP()
service for each of the AP such that the algorithm complexity
becomes O(n) * O(m) where m is the AP count.
As next steps,
1. PROCEDURE_TOKEN.Used field can be optimized out because
all tokens before FirstFreeToken should have "Used" set while all
after FirstFreeToken should have "Used" cleared.
2. ResetTokens() can be optimized to only reset tokens before
FirstFreeToken.
v2: add missing line in InitializeDataForMmMp.
v3: update copyright year to 2020.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2627
The commit will introduce a static PCD to specify the periodic interval
for checking the AP status when MP services StartupAllAPs() and
StartupThisAP() are being executed in a non-blocking manner. Or in other
words, specifies the interval for callback function CheckApsStatus().
The purpose is to provide the platform owners with the ability to choose
the proper interval value to trigger CheckApsStatus() according to:
A) The number of processors in the system;
B) How MP services (StartupAllAPs & StartupThisAP) being used.
Setting the PCD to a small value means the AP status check callback will
be triggered more frequently, it can benefit the performance for the case
when the BSP uses WaitForEvent() or uses CheckEvent() in a loop to wait
for AP(s) to complete the task, especially when the task can be finished
considerably fast on AP(s).
An example is within function CpuFeaturesInitialize() under
UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c,
where BSP will perform the same task with APs and requires all the
processors to finish the task before BSP proceeds to its next task.
Setting the PCD to a big value, on the other hand, can reduce the impact
on BSP by the time being consumed in CheckApsStatus(), especially when the
number of processors is huge so that the time consumed in CheckApsStatus()
is not negligible.
The type of the PCD is UINT32, which means the maximum possible interval
value can be set to:
4,294,967,295 microseconds = 4,295 seconds = 71.58 minutes = 1.19 hours
which should be sufficient for usage.
For least impact, the default value of the new PCD will be the same with
the current interval value. It will be set to 100,000 microseconds, which
is 100 milliseconds.
Unitest done:
A) OS boot successfully;
B) Use debug message to confirm the 'TriggerTime' parameter for the
'SetTimer' service is the same before & after this patch.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Brian J. Johnson <brian.johnson@hpe.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The "ACPI_CPU_DATA.NumberOfCpus" field is specified as follows, in
"UefiCpuPkg/Include/AcpiCpuData.h" (rewrapped for this commit message):
//
// The number of CPUs. If a platform does not support hot plug CPUs,
// then this is the number of CPUs detected when the platform is booted,
// regardless of being enabled or disabled. If a platform does support
// hot plug CPUs, then this is the maximum number of CPUs that the
// platform supports.
//
The InitializeCpuBeforeRebase() and InitializeCpuAfterRebase() functions
in "UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c" try to restore CPU configuration on
the S3 Resume path for *all* CPUs accounted for in
"ACPI_CPU_DATA.NumberOfCpus". This is wrong, as with CPU hotplug, not all
of the possible CPUs may be present at the time of S3 Suspend / Resume.
The symptom is an infinite wait.
Instead, the "mNumberOfCpus" variable should be used, which is properly
maintained through the EFI_SMM_CPU_SERVICE_PROTOCOL implementation (see
SmmAddProcessor(), SmmRemoveProcessor(), SmmCpuUpdate() in
"UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c").
When CPU hotplug is disabled, "mNumberOfCpus" is constant, and equals
"ACPI_CPU_DATA.NumberOfCpus" at all times.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200226221156.29589-3-lersek@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
[lersek@redhat.com: shut up UINTN->UINT32 warning from Windows VS2019 PR]
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2556
This patch uses CPUID signature check to skip reading the PlatformId MSR,
which is not implemented on AMD processors.
The PlatformId is used for loading microcode patches, which is also not
supported and AMD-based platforms. To mitigate the PlatformId dependency,
PcdCpuMicrocodePatchAddress and PcdCpuMicrodePatchRegionSize must be set
to 0 (default value), in order to bypass microcode loading code paths.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Match data type and format specifier for printing.
1. Type cast ProcessorNumber and FeatureIndex to UINT32
as %d only expects a UINT32.
2. Use %08x instead of %08lx for CacheControl to print Index
as it is UINT32 type.
3. Use %016lx instead of %08lx for MemoryMapped to print
(Index | LShiftU64 (HighIndex, 32)) as it is UINT64 type.
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>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Commit c7c964b and dd01704 add header file for FIT table and update
MpInitLib to support FIT based microcode shadow operation. There are
comments that FIT is Intel specific specification instead of industry
standard, which should not be placed in EDK2 MdePkg and UefiCpuPkg.
So this patch adds a platform PPI for the microcode shadow logic, and
remove the FIT related code from EDK2.
The FIT based microcode shadow support will be implemented as a new
platform PEIM in IntelSiliconPkg in edk2-platforms.
This patch doesn't provide a DXE version shadow microcode protocol,
a platform which only uses DxeMpInitLib instance only supports PCD
based microcode shadowing.
A detailed design doc can be found here:
https://edk2.groups.io/g/devel/files/Designs/2020/0214/Support%20
the%202nd%20Microcode%20FV%20Flash%20Region.pdf
TEST: Tested on FIT enabled platform.
BZ: https://tianocore.acgmultimedia.com/show_bug.cgi?id=2449
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1584
The flow of CPU feature initialization logic is:
1. BSP calls GetConfigDataFunc() for each thread/AP;
2. Each thread/AP calls SupportFunc() to detect its own capability;
3. BSP calls InitializeFunc() for each thread/AP.
There is a design gap in step #3. For a package scope feature that only
requires one thread of each package does the initialization operation,
what InitializeFunc() currently does is to do the initialization
operation only CPU physical location Core# is 0.
But in certain platform, Core#0 might be disabled in hardware level
which results the certain package scope feature isn't initialized at
all.
The patch adds a new field First to indicate the CPU's location in
its parent scope.
First.Package is set for all APs/threads under first package;
First.Core is set for all APs/threads under first core of each
package;
First.Thread is set for the AP/thread of each core.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1366
Commit b3c71b472d supported MSR setting
in different scopes. It added below macro:
CPU_FEATURE_THREAD_BEFORE
CPU_FEATURE_THREAD_AFTER
CPU_FEATURE_CORE_BEFORE
CPU_FEATURE_CORE_AFTER
CPU_FEATURE_PACKAGE_BEFORE
CPU_FEATURE_PACKAGE_AFTER
And it re-interpreted CPU_FEATURE_BEFORE as CPU_FEATURE_THREAD_BEFORE
and CPU_FEATURE_AFTER as CPU_FEATURE_THREAD_AFTER.
This patch retires CPU_FEATURE_BEFORE and CPU_FEATURE_AFTER
completely.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2465
Commit 89164babec:
UefiCpuPkg/MpInitLib: don't shadow the microcode patch twice.
attempted to use 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress'
fields to avoid loading the microcode patches data into memory again in
the DXE phase.
However, the CPU_MP_DATA structure has members with type 'UINTN' or
pointer before the microcode patch related fields. This may cause issues
when PEI and DXE are of different archs (e.g. PEI - IA32, DXE - x64),
since the microcode patch related fields will have different offsets in
the CPU_MP_DATA structure.
Commit 88bd066166:
UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA
tried to resolve the above-mentioned issue by relocating the fields
'MicrocodePatchRegionSize' and 'MicrocodePatchAddress' before members with
different size between different archs. But it failed to take the case of
pre-built binaries (e.g. FSP) into consideration.
Binaries can be built when the code base had a different version of the
CPU_MP_DATA structure definition. This may cause issues when accessing
these microcode patch related fields, since their offsets are different
(between PEI phase in the binaries and DXE phase in current code
implementation).
This commit will use the newly introduced EDKII microcode patch HOB
instead for the DXE phase to get the information of the loaded microcode
patches data done in the PEI phase. And the 'MicrocodePatchRegionSize' and
'MicrocodePatchAddress' fields in CPU_MP_DATA will not be used to pass
information between phases.
For pre-built binaries, they can be classified into 3 types with regard to
the time when they are being built:
A. Before commit 89164babec
(In other words, 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress'
were not being used to skip microcode load in DXE)
For this case, the EDKII microcode patch HOB will not be produced. This
commit will load the microcode patches data again in DXE. Such behavior is
the same with the code base back then.
B. After commit 89164babec, before commit e1ed55738e
(In other words, 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress'
being used to skip microcode load in DXE, but failed to work properly
between differnt archs.)
For this case, the EDKII microcode patch HOB will not be produced as well.
This commit will also load the microcode patches data again in DXE.
But since commit 89164babec failed to keep the detection and application
of microcode patches working properly in DXE after skipping the load, we
fall back to the origin behavior (that is to load the microcode patches
data again in DXE).
C. After commit e1ed55738e
(In other words, EDKII microcode patch HOB will be produced.)
For this case, it will have the same behavior with the BIOS built from
the current source codes.
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
This reverts commit 88bd066166.
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2465
Commit 88bd066166 relocates the 'MicrocodePatchAddress' and
'MicrocodePatchRegionSize' fields in structure CPU_MP_DATA to ensure that
they can be properly passed between different architectures.
However, such change is not backward compatible with the scenario like
pre-existing binaries such as FSP. These binaries are built when the code
base has a different version of the CPU_MP_DATA structure definition. This
may cause issues when accessing the 'MicrocodePatchAddress' and
'MicrocodePatchRegionSize' fields, since their offsets are different
(between PEI phase in the FSP binaries and DXE phase in current code
implementation).
Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Fix various typos in comments and documentation.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-79-philmd@redhat.com>
Fix various typos in comments and documentation.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-78-philmd@redhat.com>
Fix few typos in comments and documentation.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Antoine Coeur <coeur@gmx.fr>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
Message-Id: <20200207010831.9046-77-philmd@redhat.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2498
Commit fd30b00707 updated the logic in function MicrocodeDetect() that
will directly use the CPUID and PlatformID information from the 'CpuData'
field in the CPU_MP_DATA structure, instead of collecting these
information for each processor via AsmCpuid() and AsmReadMsr64() calls
respectively.
At that moment, this approach worked fine for APs. Since:
a) When the APs are waken up for the 1st time (1st MpInitLibInitialize()
entry at PEI phase), the function InitializeApData() will be called for
each AP and the CPUID and PlatformID information will be collected.
b) During the 2nd entry of MpInitLibInitialize() at DXE phase, when the
APs are waken up again, the function InitializeApData() will not be
called, which means the CPUID and PlatformID information will not be
collected. However, the below logics in MicrocodeDetect() function:
CurrentRevision = GetCurrentMicrocodeSignature ();
IsBspCallIn = (ProcessorNumber == (UINTN)CpuMpData->BspNumber) ? TRUE : FALSE;
if (CurrentRevision != 0 && !IsBspCallIn) {
//
// Skip loading microcode if it has been loaded successfully
//
return;
}
will ensure that the microcode detection and application will be
skipped due to the fact that such process has already been done in the
PEI phase.
But after commit 396e791059, which removes the above skip loading logic,
the CPUID and PlatformID information on APs will be used upon the 2nd
entry of the MpInitLibInitialize(). But since the CPUID and PlatformID
information has not been collected, it will bring issue to the microcode
detection process.
This commit will update the logic in MicrocodeDetect() back to always
collecting the CPUID and PlatformID information explicitly.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2474
Previous commit d786a17232:
UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
Removed the below assignments for the 'InitFlag' field of CPU_MP_DATA
structure in function MpInitLibInitialize() when APs are waken up to do
some initialize sync:
CpuMpData->InitFlag = ApInitReconfig;
...
CpuMpData->InitFlag = ApInitDone;
The above commit mistakenly assumed the 'InitFlag' field will have a value
of 'ApInitDone' when the APs have been successfully waken up before. And
since there is no explicit comparision for the 'InitFlag' field with the
'ApInitReconfig' value. The commit removed those assignments.
However, under some cases (e.g. when variable OldCpuMpData is not NULL,
which means function CollectProcessorCount() will not be called), removing
the above assignments will left the 'InitFlag' field being uninitialized
with a value of 0, which is a invalid value for the type of 'InitFlag'
(AP_INIT_STATE).
It may potentially cause the WakeUpAP() function to run some unnecessary
codes when the APs have been successfully waken up before:
if (CpuMpData->WakeUpByInitSipiSipi ||
CpuMpData->InitFlag != ApInitDone) {
ResetVectorRequired = TRUE;
AllocateResetVector (CpuMpData);
FillExchangeInfoData (CpuMpData);
SaveLocalApicTimerSetting (CpuMpData);
}
This commit will address the above-mentioned issue.
Test done:
* OS boot on a real platform with multi processors
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
In commit 4eee0cc7cc ("UefiCpuPkg/PiSmmCpu: Enable 5 level paging when
CPU supports", 2019-07-12), the Page Directory Entry setting was regressed
(corrupted) when splitting a 2MB page to 512 4KB pages, in the
InitPaging() function.
Consider the following hunk, displayed with
$ git show --function-context --ignore-space-change 4eee0cc7cc
> //
> // If it is 2M page, check IsAddressSplit()
> //
> if (((*Pd & IA32_PG_PS) != 0) && IsAddressSplit (Address)) {
> //
> // Based on current page table, create 4KB page table for split area.
> //
> ASSERT (Address == (*Pd & PHYSICAL_ADDRESS_MASK));
>
> Pt = AllocatePageTableMemory (1);
> ASSERT (Pt != NULL);
>
> + *Pd = (UINTN) Pt | IA32_PG_RW | IA32_PG_P;
> +
> // Split it
> - for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof(*Pt); PtIndex++) {
> - Pt[PtIndex] = Address + ((PtIndex << 12) | mAddressEncMask | PAGE_ATTRIBUTE_BITS);
> + for (PtIndex = 0; PtIndex < SIZE_4KB / sizeof(*Pt); PtIndex++, Pt++) {
> + *Pt = Address + ((PtIndex << 12) | mAddressEncMask | PAGE_ATTRIBUTE_BITS);
> } // end for PT
> *Pd = (UINT64)(UINTN)Pt | mAddressEncMask | PAGE_ATTRIBUTE_BITS;
> } // end if IsAddressSplit
> } // end for PD
First, the new assignment to the Page Directory Entry (*Pd) is
superfluous. That's because (a) we set (*Pd) after the Page Table Entry
loop anyway, and (b) here we do not attempt to access the memory starting
at "Address" (which is mapped by the original value of the Page Directory
Entry).
Second, appending "Pt++" to the incrementing expression of the PTE loop is
a bug. It causes "Pt" to point *right past* the just-allocated Page Table,
once we finish the loop. But the PDE assignment that immediately follows
the loop assumes that "Pt" still points to the *start* of the new Page
Table.
The result is that the originally mapped 2MB page disappears from the
processor's view. The PDE now points to a "Page Table" that is filled with
garbage. The random entries in that "Page Table" will cause some virtual
addresses in the original 2MB area to fault. Other virtual addresses in
the same range will no longer have a 1:1 physical mapping, but be
scattered over random physical page frames.
The second phase of the InitPaging() function ("Go through page table and
set several page table entries to absent or execute-disable") already
manipulates entries in wrong Page Tables, for such PDEs that got split in
the first phase.
This issue has been caught as follows:
- OVMF is started with 2001 MB of guest RAM.
- This places the main SMRAM window at 0x7C10_1000.
- The SMRAM management in the SMM Core links this SMRAM window into
"mSmmMemoryMap", with a FREE_PAGE_LIST record placed at the start of the
area.
- At "SMM Ready To Lock" time, PiSmmCpuDxeSmm calls InitPaging(). The
first phase (quoted above) decides to split the 2MB page at 0x7C00_0000
into 512 4KB pages, and corrupts the PDE. The new Page Table is
allocated at 0x7CE0_D000, but the PDE is set to 0x7CE0_E000 (plus
attributes 0x67).
- Due to the corrupted PDE, the second phase of InitPaging() already looks
up the PTE for Address=0x7C10_1000 in the wrong place. The second phase
goes on to mark bogus PTEs as "NX".
- PiSmmCpuDxeSmm calls SetMemMapAttributes(). Address 0x7C10_1000 is at
the base of the SMRAM window, therefore it happens to be listed in the
SMRAM map as an EfiConventionalMemory region. SetMemMapAttributes()
calls SmmSetMemoryAttributes() to mark the region as XP. However,
GetPageTableEntry() in ConvertMemoryPageAttributes() fails -- address
0x7C10_1000 is no longer mapped by anything! -- and so the attribute
setting fails with RETURN_UNSUPPORTED. This error goes unnoticed, as
SetMemMapAttributes() ignores the return value of
SmmSetMemoryAttributes().
- When SetMemMapAttributes() reaches another entry in the SMRAM map,
ConvertMemoryPageAttributes() decides it needs to split a 2MB page, and
calls SplitPage().
- SplitPage() calls AllocatePageTableMemory() for the new Page Table,
which takes us to InternalAllocMaxAddress() in the SMM Core.
- The SMM core attempts to read the FREE_PAGE_LIST record at 0x7C10_1000.
Because this virtual address is no longer mapped, the firmware crashes
in InternalAllocMaxAddress(), when accessing (Pages->NumberOfPages).
Remove the useless assignment to (*Pd) from before the loop. Revert the
loop incrementing and the PTE assignment to the known good version.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1789335
Fixes: 4eee0cc7cc
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
The existing MpInitLib will shadow the microcode update patches from
flash to memory and this is done by searching microcode region specified
by PCD PcdCpuMicrocodePatchAddress and PcdCpuMicrocodePatchRegionSize.
This brings a limition to platform FW that all the microcode patches must
be placed in one continuous flash space.
This patch shadows microcode update according to FIT microcode entries if
it's present, otherwise it will fallback to original logic (by PCD).
A new featured PCD gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit
is added for enabling/disabling this support.
TEST: Tested on FIT enabled platform.
BZ: https://tianocore.acgmultimedia.com/show_bug.cgi?id=2449
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>